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

Accept 202 http status code (instead throwing exception). #102

Merged
merged 1 commit into from Oct 8, 2016
Merged

Accept 202 http status code (instead throwing exception). #102

merged 1 commit into from Oct 8, 2016

Conversation

robocik
Copy link

@robocik robocik commented Oct 7, 2016

This status is used by WCF IsOneWay operations

@Kisty
Copy link

Kisty commented Oct 7, 2016

That sounds fair. Checking the OkHttp3 library, a response is counted as a success if the response is:

200 <= code && code < 300

Do you think you could change the status check to that?

https://github.com/square/okhttp/blob/7135628c645892faf1a48a8cff464e0ed4ad88cb/okhttp/src/main/java/okhttp3/Response.java#L105

if(is!=null)
{
parseResponse(envelope, is,retHeaders);
}
Copy link

Choose a reason for hiding this comment

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

Is the null check needed here?

@robocik
Copy link
Author

robocik commented Oct 7, 2016

To change success condition to code >= 200 && code < 300 we need to make a bigger investigation. So as a safer solution I would merge the current pull request and later if we ensure that we can use your condition, then we will create a new pull request.

We have to check for null becaues during my test when WCF service returns from IsOneWay method, then response length was 0 and the response stream was null. Which of course throws exception in parseResponse method.

@Kisty
Copy link

Kisty commented Oct 7, 2016

@robocik Ah right. That makes sense. Thanks for explaining :)

@mosabua
Copy link
Member

mosabua commented Oct 7, 2016

Could add some changelog entry and if necessary some documentation?

@Kisty
Copy link

Kisty commented Oct 7, 2016

@mosabua What changes would we like to be applied? The ability to accept status code 202 as well or any code which is 200 <= code && code < 300? Is there anything else we want added?

@mosabua
Copy link
Member

mosabua commented Oct 7, 2016

I can do the rest tonight if this is good to go from your perspective... @robocik @Kisty

@Kisty
Copy link

Kisty commented Oct 7, 2016

If you mean adding the broader status check and adding documentation surrounding that then yes, that's fine with me.

@mosabua mosabua merged commit 561c7e1 into simpligility:master Oct 8, 2016
mosabua added a commit that referenced this pull request Oct 8, 2016
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.

None yet

4 participants