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

If response is json embedded in soap, then error should not be thrown. Fix issue #580 #581

Merged
merged 1 commit into from
Mar 23, 2015

Conversation

Himanshu4
Copy link

Changes included:

  1. Changed console.log to debug when soap response is json to avoid unnecessary logs.
  2. Fix for issue #580 . When json is embedded in soap body, then currently it throws error. We won't return the error and send the response in body param. That way, it would be a cleaner approach for the client to distinguish it from other errors like "http error"/"soap error".

@herom
Copy link
Contributor

herom commented Feb 27, 2015

Thanks a lot for your contribution @Himanshu4 - and sorry for the long delay 😞. I'm getting the feeling of being the only contributor who cares for this project at the moment 😃

Personally, I'm totally happy with the addition of the debug library and I hope we make use of it more often so that this library can "speak about it's work" to the developer which would help a lot! So, whoever reads this and want to get a hand on pulling in debug wherever necessary/helpful, this would make a great PR 👍

In order to get your PR merged in, I also have to ask you for accompanying tests, so we can make shure that your code changes/additions don't get overwritten accidently. Please have a look at our Guidelines on Submitting a Pull Request in advance 😃

@Himanshu4
Copy link
Author

@herom : Apologies for so long delay. I have added a test wsdl in which response element is not defined as it is a json and also added a test case for the same. Combined both the previous commits into a single one. Please check and let me know if it can be merged now.

Thanks.

@herom
Copy link
Contributor

herom commented Mar 23, 2015

Awesome, thanks a lot 👍

herom added a commit that referenced this pull request Mar 23, 2015
If response is json embedded in soap, then error should not be thrown. Fix issue #580
@herom herom merged commit c0fbbac into vpulim:master Mar 23, 2015
@Himanshu4
Copy link
Author

@herom : In which version, would this change be start reflecting in the npm module.

@herom
Copy link
Contributor

herom commented Mar 23, 2015

@Himanshu4 I'm planning on releasing a new version of node-soap hopefully by Wednesday/Thursday this week 👍

diarmaidm pushed a commit to diarmaidm/node-soap that referenced this pull request Feb 3, 2016
If response is json embedded in soap, then error should not be thrown. Fix issue #580
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.

2 participants