Skip to content
This repository was archived by the owner on Jun 18, 2024. It is now read-only.

Conversation

@evpaassen
Copy link
Contributor

Currently, when an HTTP 500 is returned, an exception is raised about a missing SOAP header. This is because SOAP faults from EWS don't contain headers. Instead of trying to parse the SOAP message the web existing exception handling logic should be called here.

I also cleaned up the ServiceRequestBase related classes a little bit, before the fix could be done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Order alphabetically.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IntelliJ IDEA orders javax.* imports before java.* imports by default. I'm not sure we want to change this as long as the coding standards for this project are undefined.

intellij-import-defaults

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

@vboctor
Copy link
Contributor

vboctor commented Sep 2, 2014

+1 - will have another look once comments are addressed.

@evpaassen
Copy link
Contributor Author

Other comments are addressed.

@evpaassen
Copy link
Contributor Author

I also removed the FIXME's and created PR #27 to remove the unused and incorrect methods.

@vboctor
Copy link
Contributor

vboctor commented Sep 3, 2014

Please push the commits that address the pull request feedback so that it can be viewed as part of the pull request. Thanks.

@evpaassen
Copy link
Contributor Author

Please push the commits that address the pull request feedback so that it can be viewed as part of the pull request. Thanks.

They already are. I rebased the fixes into the commits where they belong. You can click on the outdated commits in this PR to see that the issues have been addressed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we should wrap the exception here either. The problem is the exception gets wrapped two or three times by this class and the problem with wrapping exceptions is that it makes handling the real exception much harder. Why are exceptions wrapped anyway?

@vboctor
Copy link
Contributor

vboctor commented Sep 26, 2014

Please update this pull request to merge automatically and I'll merge it.

@evpaassen
Copy link
Contributor Author

I addressed the issue you mentioned in your last comment about declaration and initialization of the request variable and rebased all commits on the latest version of master.

vboctor added a commit that referenced this pull request Oct 2, 2014
@vboctor vboctor merged commit ee52d02 into OfficeDev:master Oct 2, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants