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

Use URL and HTTP method in DefaultResponseErrorHandler #28958

Conversation

jerzykrlk
Copy link

Good evening team,

This is a followup of a previous PR from January 2021: #26480 .
Previously it was closed with a conclusion that the change bypasses a protected method:

Given the change of protected methods I'd rather avoid this change at this stage of the 5.3.x branch.

Now that Spring 6 is coming, do you think there's a chance to add this change?

Below is the original description of the PR:

======

The REST Client exception contains a preview of the body, which is great, and it helps a lot to quickly diagnose errors.

Now, the stacktrace looks like this:

Caused by: org.springframework.web.client.HttpServerErrorException$InternalServerError: 500 Server Error: [no body]
	at org.springframework.web.client.HttpServerErrorException.create(HttpServerErrorException.java:101)
	at org.springframework.web.client.DefaultResponseErrorHandler.handleError(DefaultResponseErrorHandler.java:238)
	at org.springframework.web.client.DefaultResponseErrorHandler.handleError(DefaultResponseErrorHandler.java:147)

I'd like to add the HTTP method and URL to the error, so that the stacktrace looks like this:

Caused by: org.springframework.web.client.HttpServerErrorException$InternalServerError: 500 Server Error after GET http://localhost:49709/status/server : [no body]
	at org.springframework.web.client.HttpServerErrorException.create(HttpServerErrorException.java:101)
	at org.springframework.web.client.DefaultResponseErrorHandler.handleError(DefaultResponseErrorHandler.java:238)
	at org.springframework.web.client.DefaultResponseErrorHandler.handleError(DefaultResponseErrorHandler.java:147)

======

Thank you!

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 12, 2022
@jerzykrlk jerzykrlk force-pushed the url-and-method-in-rest-client-exception branch 3 times, most recently from 3a05fad to 654d178 Compare August 23, 2022 18:28
@jerzykrlk jerzykrlk force-pushed the url-and-method-in-rest-client-exception branch from 654d178 to 51245d4 Compare August 23, 2022 18:55
@rstoyanchev rstoyanchev added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jan 24, 2023
@rstoyanchev rstoyanchev self-assigned this Jan 27, 2023
@rstoyanchev rstoyanchev added the type: enhancement A general enhancement label Jan 27, 2023
@rstoyanchev rstoyanchev removed their assignment Mar 6, 2023
Copy link
Contributor

@rstoyanchev rstoyanchev left a comment

Choose a reason for hiding this comment

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

Apologies this fell through again, but already too late for 6.1. We have looked into it however and discussed it. We plan to add it in 6.2.

One thing to keep in mind, the exception message should not include the full URI, in particular the query which may reveal sensitive info.

Also, now that we have both RestClient and RestTemplate, for extra context it's worth mentioning that while RestClient makes it possible to use ResponseErrorHandler, it also exposes its own ErrorHandler interface that takes the request and response.

@rstoyanchev rstoyanchev removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 24, 2023
@rstoyanchev rstoyanchev added this to the 6.2.x milestone Nov 24, 2023
@rstoyanchev rstoyanchev changed the title URL and HTTP method in rest client exception message Use URL and HTTP method in DefaultResponseErrorHandler Nov 24, 2023
@jerzykrlk
Copy link
Author

jerzykrlk commented Nov 27, 2023

Hello Rossen!

I'll resolve the conflicts then.

I haven't followed this very closely. So let me ask a silly question - is there a chance that the behaviour stays consistent across WebClient, RestTemplate and RestClient?

And one more - would you be open to more fine-tuning in the future?

Thanks!

@snicoll snicoll self-assigned this Apr 30, 2024
@snicoll snicoll modified the milestones: 6.2.x, 6.2.0-M2 Apr 30, 2024
@snicoll snicoll closed this in a7ceedf Apr 30, 2024
@snicoll
Copy link
Member

snicoll commented Apr 30, 2024

Thanks very much @jerzykrlk! I've polished things up a bit to streamline thing. I've also changed the structure of the error message to be more compliant with other places in the framework. It now reads:

500 Server Error on GET request for "http://localhost:49709/status/server": [no body]

@jerzykrlk
Copy link
Author

Hello Stéphane - thanks for accepting this!

@jerzykrlk jerzykrlk deleted the url-and-method-in-rest-client-exception branch April 30, 2024 10:35
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) type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants