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

More error details in RestTemplate client and server exception [SPR-17130] #21667

Closed
spring-projects-issues opened this issue Aug 6, 2018 · 6 comments
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Aug 6, 2018

Jerzy Krolak opened SPR-17130 and commented

The default RestTemplate HTTP error exceptions might contain some preview of the response body, along with the HTTP status code.

So that, instead of this:

org.springframework.web.client.HttpClientErrorException.BadRequest: 400 Bad Request
    at org.springframework.web.client.DefaultResponseErrorHandler.handleError(DefaultResponseErrorHandler.java:91) ~[spring-web-4.2.4.RELEASE.jar:4.2.4.RELEASE]
    at org.springframework.web.client.RestTemplate.handleResponse(RestTemplate.java:641) ~[spring-web-4.2.4.RELEASE.jar:4.2.4.RELEASE]

... I could see this:

org.springframework.web.client.HttpClientErrorException.BadRequest: 400 Bad Request [<html><head><title>page not found</title>...]
    at org.springframework.web.client.DefaultResponseErrorHandler.handleError(DefaultResponseErrorHandler.java:91) ~[spring-web-4.2.4.RELEASE.jar:4.2.4.RELEASE]
    at org.springframework.web.client.RestTemplate.handleResponse(RestTemplate.java:641) ~[spring-web-4.2.4.RELEASE.jar:4.2.4.RELEASE]

or even this:

org.springframework.web.client.HttpClientErrorException.Conflict: 409 Conflict [{
"error": {
 "errors": [
  {
   "domain": "global",
   "reason": "conflict",
   "message": "You already own this bucket. Please select another name."
  }
 ],
 "code": 409,
 "message": "You already own this bucket. Please select another name."
 }
}]
    at org.springframework.web.client.DefaultResponseErrorHandler.handleError(DefaultResponseErrorHandler.java:91) ~[spring-web-4.2.4.RELEASE.jar:4.2.4.RELEASE]
    at org.springframework.web.client.RestTemplate.handleResponse(RestTemplate.java:641) ~[spring-web-4.2.4.RELEASE.jar:4.2.4.RELEASE]

This should not introduce too much overhead, as the response is already there, in byte [] body.


Affects: 5.0.8

Issue Links:

Referenced from: pull request #1956

@spring-projects-issues
Copy link
Collaborator Author

Rossen Stoyanchev commented

Actually RestClientResponseException already has getResponseBodyAsString(). It requires explicit use but that seems appropriate since not all content types are printable.

@spring-projects-issues
Copy link
Collaborator Author

Jerzy Krolak commented

That's correct. It would require an explicit call, though.

What I would see is a preview of the response body in the message itself - so that It always goes to the application logs without extra coding effort.

This is useful to eliminate many problems. For example, "did I speak to the correct service". Or, with services that return HTTP 400 in response to many different problems.

I will try to code something this, or next week.

@spring-projects-issues
Copy link
Collaborator Author

Jerzy Krolak commented

This is the general idea:

https://github.com/jerzykrlk/spring-framework/pull/2/files

The exception message would look somewhat like this:

404 Not Found after GET http://localhost:8080/my-endpoint : [some response body]

What do you think?

@spring-projects-issues
Copy link
Collaborator Author

Jerzy Krolak commented

Hi again, I've updated the code a bit and I've added a PR directly to Spring repository:

#1956

Do you think you could find a little time for a review? Thanks.

@spring-projects-issues
Copy link
Collaborator Author

Jerzy Krolak commented

Hi again.

It's been a while now. Can you let me know if this makes sense to you? And if are going to consider such feature in the future?

I see a big value in a detailed error description in the exception message - especially in production logs. It is much easier to diagnose issues, when I can see exactly what request caused an error. I think an URI, HTTP method and a short preview of the response would be helpful.

Thanks.

@rstoyanchev
Copy link
Contributor

This is superseded by #1956.

@rstoyanchev rstoyanchev removed this from the 5.x Backlog milestone Nov 13, 2019
@rstoyanchev rstoyanchev added the status: superseded An issue that has been superseded by another label Nov 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants