Skip to content

Conversation

devinrader
Copy link
Contributor

Modifies the behavior of

fde15e2
0fe0fa1
9b423a8

which have caused a regression in RestSharp behavior.

These commits limit RestSharp to attempt to deserialize responses only when the 200, 201 and 203 status codes are returned. This is incorrect as virtually every status code allows you to return a message-body. As far as I can tell there are only two that explicitly prohibit it: 204 and 304.

I believe the original intent of these commits was to try to stop RestSharp from attempting deserialization for non-protocol errors (ex: connection timeouts). In most cases, when RestSharp encounters a non-protocol error, it will handle the error and set the ErrorException and ErrorMessage properties allowing you to detect the occurrence of the exception.

This pull request modifies the prior commits to check the ErrorException property in order to determine of deserialization should be attempted, instead of relying on Status Codes.

Tests are included to simulate RestSharp handling a connection timeout and deserialization of an 400 error message-body. If there are other non-protocol error scenarios that should be tested let me know. I also changed one test back to its original code which would have caught this regression.

@devinrader
Copy link
Contributor Author

@haacked can you review this? I've got several customers who have been broken by this regression.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation is weird.

@haacked
Copy link
Contributor

haacked commented Oct 11, 2013

:shipit: Looks like the formatting is off, but other than that, feel free to merge it once you fix that up.

devinrader pushed a commit that referenced this pull request Oct 11, 2013
All all HTTP responses to be deserialized
@devinrader devinrader merged commit 3a174c5 into restsharp:master Oct 11, 2013
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