URL and HTTP method in rest client exception message #26480

jerzykrlk · 2021-01-31T12:58:01Z

Dear team,

This is a followup of a discussion in a past PR: #1956

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!

…ated

jerzykrlk · 2021-01-31T13:00:01Z

spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java

@@ -132,9 +154,10 @@ public void handleError(ClientHttpResponse response) throws IOException {
 	 * </pre>
 	 */
 	private String getErrorMessage(


Ideally, I'd make this method protected - so that I can override it to extract some more specific information from responses that return HTML, XML or some other nonstandard data.

Considering something beyond 5.3.x - what about this one?

I'd love to have an extension point where I can further customize the response body preview - format it right, remove some tags, etc.

A long time ago, here: https://github.com/spring-projects/spring-framework/pull/1956/files - I even thought about a separate class to extract the body. But a protected method would be just fine.

rstoyanchev

Given the change of protected methods I'd rather avoid this change at this stage of the 5.3.x branch. As far as I can see you can still make it happen in your own custom extension of DefaultResponseErrorHandler, right? It just wouldn't be out of the box.

rstoyanchev · 2021-02-09T10:56:32Z

spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java

@@ -175,11 +207,29 @@ private String getErrorMessage(
 	 * @see HttpServerErrorException#create
 	 */
 	protected void handleError(ClientHttpResponse response, HttpStatus statusCode) throws IOException {
+		handleError(null, null, response, statusCode);
+	}


This is a protected method that is now bypassed. Something to consider for a minor/major release but not at this stage of the 5.3.x branch.

Thanks for looking into that!

Initially, I didn't think of squeezing it into 5.3. I'd be very happy to do so - but I think the previous changes broke something in spring-integration, so it might not be that easy.

I tried to not to change the behaviour of this method at all. I can also try to rewrite this to make it more 5.3-compatible. But If you think it makes more sense to wait - let's wait.

jerzykrlk · 2021-02-09T20:39:26Z

As far as I can see you can still make it happen in your own custom extension of DefaultResponseErrorHandler, right? It just wouldn't be out of the box.

I'm not sure if you're refering to making getErrorMessage protected, or the entire PR?

rstoyanchev · 2021-02-10T07:14:27Z

spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java

 			throw new UnknownHttpStatusCodeException(message,
 					response.getRawStatusCode(), response.getStatusText(),
 					response.getHeaders(), body, getCharset(response));
 		}
-		handleError(response, statusCode);
+		handleError(url, method, response, statusCode);


This is the protected method that's now bypassed.

Thanks for the explanation!

Then it can definitely wait for a future Spring release. I'm not in a particular hurry - any custom extensions would well enough.

Should I close the PR, or just keep it here?

Also - is it something that you'd see easy to port into WebClient? Would it make sense to make these two in sync?

I think we can close it for now. On the WebClient side it looks we do have the URL and HTTP method in WebClientResponseException.

Thanks for the feedback @rstoyanchev !

Is there a chance to squeeze this in a future Spring Framework release? Should I come back once it's announced?

Yes, of course. We can probably re-open this PR then.

rstoyanchev · 2021-02-10T07:17:02Z

As far as I can see you can still make it happen in your own custom extension of DefaultResponseErrorHandler, right? It just >> wouldn't be out of the box.

I'm not sure if you're refering to making getErrorMessage protected, or the entire PR?

I'm talking about the call to the protected handleError method that is now bypassed.

jerzykrlk added 2 commits January 30, 2021 23:15

http error details in the rest client exception message

a31eaf3

http error details in the rest client exception message - javadoc upd…

c2cdc6e

…ated

jerzykrlk commented Jan 31, 2021

View reviewed changes

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 31, 2021

qw81641493 approved these changes Feb 2, 2021

View reviewed changes

rstoyanchev reviewed Feb 9, 2021

View reviewed changes

rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) status: waiting-for-feedback We need additional information before we can continue type: enhancement A general enhancement labels Feb 9, 2021

spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Feb 9, 2021

rstoyanchev reviewed Feb 10, 2021

View reviewed changes

rstoyanchev closed this Feb 11, 2021

rstoyanchev removed status: feedback-provided Feedback has been provided status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 11, 2021

jerzykrlk mentioned this pull request Aug 12, 2022

Use URL and HTTP method in DefaultResponseErrorHandler #28958

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

URL and HTTP method in rest client exception message #26480

URL and HTTP method in rest client exception message #26480

jerzykrlk commented Jan 31, 2021

jerzykrlk Jan 31, 2021

jerzykrlk Feb 9, 2021

rstoyanchev left a comment

rstoyanchev Feb 9, 2021

jerzykrlk Feb 9, 2021

jerzykrlk commented Feb 9, 2021

rstoyanchev Feb 10, 2021

jerzykrlk Feb 10, 2021 •

edited

Loading

rstoyanchev Feb 11, 2021

jerzykrlk Feb 22, 2021

rstoyanchev Feb 25, 2021

rstoyanchev commented Feb 10, 2021

URL and HTTP method in rest client exception message #26480

URL and HTTP method in rest client exception message #26480

Conversation

jerzykrlk commented Jan 31, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rstoyanchev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jerzykrlk commented Feb 9, 2021

Choose a reason for hiding this comment

jerzykrlk Feb 10, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rstoyanchev commented Feb 10, 2021

jerzykrlk Feb 10, 2021 •

edited

Loading