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

SOAPFault wrongly assumes that the hash keys will be symbols #393

Closed

Conversation

DavidEGrayson
Copy link
Contributor

First of all, thank you for making this excellent gem.

I like to use the convert_tags_to: nil option with Savon so that it will work with some legacy code that requires the response hash keys to be CamelCase strings. However, the SOAPFault class assumes the response hash keys will be snake_case symbols, which is the default. As a result, if a SOAP fault occurs and the error handling code tries to call to_s or to_hash on the SOAPFault exception we get another exception (undefined method '[]' for nil:NilClass). This is bad because it makes it hard for me to see the actual error message returned by the server. Also, developers probably don't expect that calling to_s will raise another exception.

This pull request contains some new rspec examples in soap_fault_spec.rb that demonstrate the problem.

How do you think we should solve this? I see two reasonable options:

  1. Add a new method to Nori that is like parse, but ignores the two special options that affect keys (strip_namespaces and convert_tags_to). Use this method in the SOAPFault class so that that the keys in the hash will be predictable.
  2. Add a new method to Nori that takes a string tag name as an input, and returns the expected hash key as an output. That code already exists in the initialize method of Nori::XMLUtilityMode. Use this method in the SOAPFault class so that we can be sure to access the right hash key, not matter what it is.

I think option 1 is cleaner, but option 2 might be better because we can then use it to make Savon::Response#body and Savon::Response#header work properly. ( However I see from here that the you have already considered that and decided against it. )

Thanks!

…should be able to handle it if the nori parser is configured to convert tag names to hash keys in a non-default way.
@rubiii
Copy link
Contributor

rubiii commented Feb 9, 2013

you're right. both the soap fault as well as the response#body and response#header methods
don't respect this option. i think the spec only documents this behavior and it should be fixed.

your second option sounds like the way to go.

rubiii added a commit to savonrb/nori that referenced this pull request Apr 21, 2013
@rubiii rubiii closed this in a1fd031 Apr 21, 2013
@rubiii
Copy link
Contributor

rubiii commented Apr 21, 2013

@DavidEGrayson i followed your suggestion and pushed a fix for this to master. thanks!

@DavidEGrayson
Copy link
Contributor Author

Cool, thanks for doing that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants