Error responses (like 500) and generic RestResponse<T> class - can they coexist?

[mark Kharitonov]

Hi.
I have noticed that the following combinations blows in the face:

• Using generic RestResponse<T>
• Server returning errors, like 500 with a respective error representation
  

What happens is that the generic response class attempts to deserialize the error representation into T, which surely fails.

I do not see how can one use the generic API given this scenario must be prepared for. What do you think?

[Andrew Young]

Are your server errors returning HTML if you requested JSON? Or is it returning JSON but just a different object graph?

[Mark Kharitonov]

It returns json, but of course a different object graph.

Other web servers may return html or whatever they please.

In general I expected the method to throw the right WebException.

Envoyé de mon iPhone

[Andrew Young]

Perhaps ReshSharp should check the if the response status isn't 500 before trying to deserialize. It would be nice if it could handle those servers that are returning a different object graph for errors. Perhaps a RestResponse<T, E> or something?

The first suggestion would probably be the easiest to implement for now.

[Mark Kharitonov]

I would not sanctify the error 500. The underlying HTTP stack detects that there was an error and raises the correct WebException. The problem is that this exception is not delivered up the stack to the caller. Instead the deserializer is called and fails and then that deserialization exception becomes the exception visible to the caller. 

==========================================================================

There are two kinds of people. Those whose guns are loaded and those who dig.

(The good, the bad and the ugly).

So let us raise our cups for our guns always be loaded.

[John Sheehan]

Non-200 HTTP status codes are not exceptional in REST. 500 is a status code that indicates an error, not an error code. That's an important distinction.

I think the OnBeforeDeserialization handler should help for your situation: http://restsharp.org/follow-up-to-different-root-on-error-changes/

[mark Kharitonov]

If I demand a resource X through client.Execute<X> and it fails because of 401 Unauthorize, should the method raise an exception or return null? I think it should throw. If you agree, then 401 and 500 have to be treated the same - an exception has to be raised and we even know which exception - WebException. Moreover, we do not need to create this exception - the underlying infrastructure has already created it for us. Of course, some of its properties are crap, but it embeds the response object, which has all the info we need. Maybe we could rethrow it as a different exception, but this is details.

Now, if you disagree with me, then client.Execute<X> should return null on 401, but throw an exception on 500 (or return null as well ?). I do not see how this makes the client life easier. Yes, we are being strict about the HTTP status code nuances at the expense of the client code simplicity.

2011/8/3 John Sheehan <johns...@gmail.com>

Non-200 HTTP status codes are not exceptional in REST. 500 is a status code that indicates an error, not an error code. That's an important distinction.

I think the OnBeforeDeserialization handler should help for your situation: http://restsharp.org/follow-up-to-different-root-on-error-changes/

--
Be well and prosper.
==============================
"There are two kinds of people.Those whose guns are loaded and those who dig."
   ("The good, the bad and the ugly")
So let us drink for our guns always be loaded.

[John Sheehan]

If you want status codes to throw exceptions, RestSharp is not the library for you. You can act on non-200s before deserialization with OnBeforeDeserialization. If more hooks are needed, we can do that, but but RestSharp will never throw an exception by default based on status code.

[mark Kharitonov]

This is a new field to me and I am trying to learn. Why not?

Observer the current generic API. Here is the client code:

var x = client.Execute<X>(request);

Suppose the server returns 401 Unauthorized.
Here is the detailed account of what happens next on the client side:

1. RestSharp.Http.GetRawResponse catches the WebException and returns its response.
2. RestSharp.RestClient.Execute<T> continues straight to RestSharp.RestClient.Deserialize<T>, even though I did not ask for the RestResponse<T>.Data property.
3. But what is going to be deserialized? Is it an instance of T? No, no and no. It is the error content that is going to be deserialized as T.
4. Now, what am I supposed to do inside OnBeforeDeserialization, besides throwing an exception? Inject a fake T instance into the response content?
5. If I did nothing inside OnBeforeDeserialization (because I do not know what to do there), then deserialization fails with an exception that has nothing to do with our business logic, but that exception is caught and then what? Observe:
• response.ResponseStatus = ResponseStatus.Error; // We have just overwritten the Completed status with Error - wrong, because HTTP did complete.
• response.ErrorMessage = ex.Message; // Is it possible that we are overwriting some other, more relevant error message here?

I do not find this flow as correct. The only way for me to correct it is:

1. override the OnBeforeDeserialization method
2. recognize that the response is actually an error response and it does not matter for me here whether it is 401 or 500 - please prove me wrong.
3. do what? Throw an exception, because I cannot affect the result of the method.

Please, show me where I am wrong in my analysis.

[Andrew Young]

John,

I'd like to propose that we make the OnBeforeDeserialization a Func<RestResponse, bool> vs. the current Action<RestResponse>. The idea being that it can return a bool indicating whether it should go ahead and deserialize or false, don't deserialize. This would give end users the ability to handle certain responses however they want to without having RestSharp force deserialization in the case where a server doesn't respond with the requested content type.

[Andrew Young]

Mark, I suppose if you really wanted to throw an exception on anything other than a 200 you could check the status and throw your own exception

On Wednesday, August 3, 2011 at 1:53, mark Kharitonov wrote:

[Andrew Young]

sorry, that last one was sent prematurely.

As I was saying. You could throw your own exception in OnBeforeDeserialization. I haven't tried this out but it should work as long as nothing else is catching it up the call stack.

But my proposal to make OnBeforeDeserialization a Func should solve your problem. That way you can handle the response and error code before everything gets deserialized and just tell RestSharp to skip deserialization.

[mark Kharitonov]

Yep, making it a boolean function is a good idea.

[Andrew Young]

And here it is. https://github.com/johnsheehan/RestSharp/commit/cb57637afa1f602c354da2938b49a172bb01d4e7