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

ResponseStatus is Error when response is 404 NotFound #837

Closed
mwinder opened this issue Apr 21, 2016 · 8 comments
Closed

ResponseStatus is Error when response is 404 NotFound #837

mwinder opened this issue Apr 21, 2016 · 8 comments

Comments

@mwinder
Copy link

mwinder commented Apr 21, 2016

Use RestClient.Execute(...) to perform a GET that results in a 404.
The IRestResponse that comes back has ResponseStatus = Error.
- The ErrorException property has the message "Invalid JSON string".
- The Data property response = null.

Perform the same request with the (non-generic) RestClient.Execute(...).
The IRestResponse has ResponseStatus = Complete

I appears that it is failing to deserialize the response into the typed model (TResult).
In this particular API resource returns a Content body of " " (single space character), not sure if it should be a completely empty body.

I would have expected the ResponseStatus in both cases to be Completed - the request succeeded, there where no transport errors.

@behoyh
Copy link

behoyh commented May 24, 2016

I have set up a Windows Forms App to test. Could you provide more details about this issue? I'm not sure what you mean by "non-generic"

@mwinder
Copy link
Author

mwinder commented May 25, 2016

By "non-generic" I meant the Execute method on RestClient that doesn't take a generic parameter:

IRestResponse Execute(IRestRequest request);

versus:

IRestResponse<T> Execute<T>(IRestRequest request) where T : new();

If it helps I ended up working around the issue by writing an extension method on IRestResponse that uses Json.NET to deserialize the Content property:

public static TResult Content<TResult>(this IRestResponse response)
{
    return JsonConvert.DeserializeObject<TResult>(response.Content);
}

I could then use the "non-generic" Execute method to make my request and get a typed response with my extension method:

var resp = client.Execute(new RestRequest("some/resource"));

if (resp.ResponseStatus != ResponseStatus.Completed) { // Check whether request completed
     //Handle error
}

if (resp.StatusCode == HttStatusCode.NotFound) { // Check for 404
     //Handle 404
}

var result = resp.Content<SomeResult>();

Performing the same steps with the "generic" Execute:

var resp = client.Execute<SomeResult>(new RestRequest("some/resource"));

if (resp.ResponseStatus != ResponseStatus.Completed) {
     //This is reached even for 404 - I presume because of a failure during deserialization
}

//Handling 404 here isn't possible be cause it appears the request did not complete...
if (resp.StatusCode == HttStatusCode.NotFound) {
    //Handle 404
}

var result = resp.Data; // Data is null

@simonob
Copy link

simonob commented Jun 16, 2016

I'm getting this same problem using v105.2.3, so have changed my Execute<> calls to Execute and used the suggestion above which has resolved the issue.

The issue appears to be due to the content from the server for an HTTP 500 response being a chunk of HTML, which can't be deserialized into my type using the JSON serializer and therefore I'm guessing the ResponseStatus is set to Error as the deserialization failed.

The current documentation suggests that the ResponseStatus is only set for network/connectivity-related errors, and not deserialization errors.

For what it's worth, my proposal would be that the Data property throws an exception if deserialization fails, rather than changing the ResponseStatus from Completed to Error (although this may represent a breaking change for those expecting it to work as it does currently).

@benbward
Copy link

benbward commented Mar 21, 2018

Still open? Is this going anywhere? Seems like it should either be addressed or commented on & closed by now.

@alexeyzimarev
Copy link
Member

I think it should be fixed. The suggestion of @simonob sounds very reasonable. Many apps return crappy responses that we cannot deserialise, this is quite normal.

@alexeyzimarev
Copy link
Member

Ok, that it in fact by design. I added an option to return Completed when deserialization fails. For the backward compatibility, I must keep current behaviour as-is, but if you set RestClient.FailOnDeserializationError to false in the next version, you will get it sorted.

@forbjok
Copy link

forbjok commented Jan 29, 2020

Out of curiousity, what was the reason for going with this behavior?
To me, it seems like the most logical behavior in the case where a request successfully completes and receives a status code from the server, but fails to be deserialized locally, would be to have IsSuccessful be false and ResponseStatus = Completed.

The way it is currently, there is as far as I can tell no way to tell from looking at either of those properties, whether the request failed to get a response from the server, or completed successfully but failed to deserialize. They basically both seem to indicate the same thing, making one of them redundant.

@Zunair
Copy link

Zunair commented Feb 2, 2023

I had a similar issue - so I tested the same get request on PostMan.
Get request worked from browser but not on PostMan and from RestSharp.
Compared the request headers - found that Accept-Language was required for api(phpMyFaq) I was using.

Added the missing header in the request - all looks good on PostMan and RestSharp.

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

No branches or pull requests

8 participants