Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Malformed Faults fail in non-informative ways #79

Closed
wants to merge 1 commit into from

Conversation

hartsock
Copy link
Member

example: LicenseUsageFault

  <soapenv:Body>
    <soapenv:Fault>
      <faultcode>ServerFaultCode</faultcode>
      <faultstring/>
      <detail>
        <LicenseUsageFaultFault xsi:type="LicenseUsageFault">
           <reason>dataTampered</reason></LicenseUsageFaultFault>
      </detail>
    </soapenv:Fault>

The detail tag's first child represents the data type to unmarshal the Fault message envelope's contents into. If that child tag does not have a matching WSDL definition pyVmomi fails to raise the fault. The exception raised is KeyError and contains no details about the fault. Instead, an end user should see additional information so they can report the bug accurately.

This change lets the library dynamically create the new fault data type at runtime.

closes #72

example: LicenseUsageFault
  <soapenv:Body>
    <soapenv:Fault>
      <faultcode>ServerFaultCode</faultcode>
      <faultstring/>
      <detail>
        <LicenseUsageFaultFault xsi:type="LicenseUsageFault">
           <reason>dataTampered</reason></LicenseUsageFaultFault>
      </detail>
    </soapenv:Fault>

The detail tag's first child represents the data type to unmarshal the Fault message envelope's contents into. If that child tag does not have a matching WSDL definition pyVmomi fails to raise the fault.

This change lets the library dynamically create the new fault data type at runtime.

closes vmware#72
@michaelrice
Copy link
Contributor

impressive sir. 👍

@hartsock hartsock changed the title Undocumented Faults incorrectly de-serialize Malformed Faults fail in non-informative ways Jul 23, 2014
@hartsock
Copy link
Member Author

So far, general feedback I've gotten is this is not the correct approach. I will attempt a different fix for this that will raise a much more informative traceback with the SOAP message details inside it. The goal would be for the traceback at the top most level to include the SOAP XML that the parser could not handle.

@hartsock
Copy link
Member Author

Part of the re-work of this patch slipped in here: 4ab4560

This means we need better coverage than what we have since it should not have been so easy to slide in code that would break the lib in edge-cases like that.

@hartsock hartsock closed this Jul 29, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Malformed Faults fail in non-informative ways
2 participants