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

Make it easier for RestTemplate to deal with error object responses [SPR-10961] #15589

Closed
spring-issuemaster opened this issue Oct 7, 2013 · 13 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Oct 7, 2013

Willie Wheeler opened SPR-10961 and commented

One challenge with RestTemplate is that it expects a single response type when making calls. Of course we get to choose it, but there's only one per call. The problem is that some APIs (e.g. Github API as one example) return either the expected type or else an error object, depending on whether there was an error.

It would be great if we could tell RestTemplate to deserialize into such-and-such class if there's no error, and into some other class if there is an error.

Right now the workaround I'm using is to deserialize into a String, and then parse it into the correct type depending on the status code. This is OK, but it's kind of a pain (see the blog post above), and worse, it requires buffering the entire response payload as a string, which may not be great if the payloads are large.


Affects: 4.0 M3

Reference URL: http://springinpractice.com/2013/10/07/handling-json-error-object-responses-with-springs-resttemplate/

Issue Links:

  • #20103 RestTemplate is missing "Typed" error handler. ("is superseded by")

30 votes, 25 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 11, 2013

Phil Webb commented

Is it possible to obtains the error response directly from HttpStatusCodeException.getResponseBodyAsString or write a custom ResponseErrorHandler to deserialize the error response but encapsulate it within the exception?

try {
    DoodadResources response = restTemplate.whatever...
}
catch (MyErrorResourceException ex) {
    ex.getCode();
    ex.getWhateverFromTheErrorResponse();
}
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 11, 2013

Willie Wheeler commented

If you mean that the call ex.getWhateverFromTheErrorResponse() would handle the deserialization behind the scenes--e.g.

public class HttpStatusCodeException {

    public ErrorObject getErrorObject() { ... read input stream and deserialize into ErrorObject ... }
}

then yeah, that would be very convenient from an API user's perspective. I like it.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 11, 2013

Phil Webb commented

Do the existing ResponseErrorHandler and/or HttpStatusCodeException classes provide all you need to do that? Is there still some improvement that you are looking for in the core Spring framework?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 11, 2013

Willie Wheeler commented

Well, the one thing though is that there's no standard error object format. So "ErrorObject" wouldn't be something that Spring provides--it would need to reflect the schema of the API that generated it (e.g. GitHub, HipChat, etc.) So I think that you end up still needing to pass a type param into the RestClient call to specify the return type for the error object. Now if RestTemplate wants to pass that along to HttpStatusCodeException so that HttpStatusCodeException can return an appropriately typed error object, superb.

I do like the idea though of exposing the object through the exception itself. That makes a lot of sense to me.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 11, 2013

Willie Wheeler commented

OK, I think I understand better your suggestion. You are saying that I implement ResponseErrorHandler and get the error body ClientHttpResponse.getStatusText(). Then I throw a custom exception that exposes the deserialized exception.

That is definitely superior to my current always-deserialize-to-String approach, so I will start doing that. Thanks.

Having said that, I think it would still be a useful enhancement to RestTemplate itself if people didn't have to do this. By now I think error objects are sufficiently common (GitHub and HipChat have them, for example) that it would be nice if RestTemplate just allowed the user to provide the error type as a type param (passed along to HttpStatusCodeException). Then HttpStatusCodeException itself would expose the error object. Pretty hands-free for a common requirement.

Thanks.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 11, 2013

Phil Webb commented

(sorry I was typing this before I saw your followups)

I was thinking more that you would write a specific ResponseErrorHandler to deal with the errors that relate to a specific API. Passing an additional type to the RestClient call doesn't buy you much since you still need to catch specific exception.

With a custom ResponseErrorHandler you could write client code like this:

RestTemplate t = new RestTemplate();
t.setErrorHandler(new HipChatErrorHandler());
try {
    t.exchan...
}
catch (NastyHipChatException ex) {
    ex.somethingSpecificExtractedFromTheResponse()
}
catch (OtherNastyHipChatException ex) {
    ex.somethingElseSpecificExtractedFromTheResponse()
}

You could even create your own exception hierarchy as needed.

BTW: Have you seen the Spring Social GitHub bindings?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 11, 2013

Phil Webb commented

Just so I understand you mean something like this:

try {
    HipChatResponse r = restTemplate.postForObject(uri, request, HipChatResponse.class, HipChatError.class);
}
catch (HttpErrorResponseException ex) {
   HipChatError error = (HipChatError) ex.getErrorObject();
}
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 11, 2013

Willie Wheeler commented

Yup Phil, that's right.

Re: Spring Social GitHub:
https://github.com/spring-projects/spring-social-github/graphs/contributors

It's been a while though. :-)

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 18, 2013

Phil Webb commented

Given that current design will allow you to deal with this via ResponseErrorHandler and HttpStatusCodeException (albeit not as nicely as you suggest) I am going to mark this as "Contributions Welcome" for now and see how many votes/watches we get.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 26, 2017

Geoffrey Wiseman commented

Handling this through an exception is a viable workaround, but it's clumsy, and not well-suited to handling a wide array of REST APIs (IMO)

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 26, 2017

Rossen Stoyanchev commented

Geoffrey Wiseman, in 5.0 we have added the WebClient which allows you to perform an exchange() and get access to the response status and headers, before the response body is consumed or decoded. So you can check the status first, and then decide how to proceed. It also provides a way a convenience route to retrieve() and convert to a body with pre-declared status handlers that indicate how errors should be treated.

As for the RestTemplate, what is the alternative? Given its API where you specify the target type upfront, the only thing we can do is return that type, assuming success. If there is an error, the error response will be a different type and all we can do at that point is raise an exception.

Note that in 5.0 we have added a dedicated ResponseErrorHandler for doing just that, see #20103. So it looks like we can close this ticket.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 26, 2017

Geoffrey Wiseman commented

I admit, your hands are a little tied by the existing RestTemplate API. RestTemplate is more convenient than writing HTTP client code, but its assumptions often don't line up that well with the way I want to write services.

It does looks like WebClient may be better, although at the moment I'd have to go to Spring Boot 2.0.0M5 to find out.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 26, 2017

Rossen Stoyanchev commented

Yes in more than one respects WebClient is a more modern alternative. We've deprecated AsyncRestTemplate in favor of WebClient. The RestTemplate is not deprecated and is fine to use if it meets the needs but WebClient does more with no downsides.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.