Skip to content

Commit

Permalink
Polish "Include URL and HTTP method in DefaultResponseErrorHandler"
Browse files Browse the repository at this point in the history 
See gh-28958
  • Loading branch information
@snicoll
snicoll committed Apr 30, 2024
1 parent 4972d18 commit f85c4e1
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 68 deletions.
82 changes: 38 additions & 44 deletions spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -131,15 +131,17 @@ protected boolean hasError(int statusCode) {
* {@link HttpStatus} enum range.
* </ul>
* @throws UnknownHttpStatusCodeException in case of an unresolvable status code
* @see #handleError(URI, HttpMethod, ClientHttpResponse, HttpStatusCode)
* @see #handleError(ClientHttpResponse, HttpStatusCode, URI, HttpMethod)
*/
@Override
public void handleError(ClientHttpResponse response) throws IOException {
handleError(null, null, response);
HttpStatusCode statusCode = response.getStatusCode();
handleError(response, statusCode, null, null);
}

/**
* Handle the error in the given response with the given resolved status code.
* Handle the error in the given response with the given resolved status code
* and extra information providing access to the request URL and HTTP method.
* <p>The default implementation throws:
* <ul>
* <li>{@link HttpClientErrorException} if the status code is in the 4xx
Expand All @@ -152,58 +154,51 @@ public void handleError(ClientHttpResponse response) throws IOException {
* {@link HttpStatus} enum range.
* </ul>
* @throws UnknownHttpStatusCodeException in case of an unresolvable status code
* @see #handleError(URI, HttpMethod, ClientHttpResponse, HttpStatusCode)
* @since 6.2
* @see #handleError(ClientHttpResponse, HttpStatusCode, URI, HttpMethod)
*/
@Override
public void handleError(URI url, HttpMethod method, ClientHttpResponse response) throws IOException {
HttpStatusCode statusCode = response.getStatusCode();
handleError(url, method, response, statusCode);
handleError(response, statusCode, url, method);
}

/**
* Return error message with details from the response body. For example:
* <pre>
* 404 Not Found: [{'id': 123, 'message': 'my message'}]
* 404 Not Found on GET request for "https://example.com": [{'id': 123, 'message': 'my message'}]
* </pre>
*/
private String getErrorMessage(
int rawStatusCode, String statusText, @Nullable byte[] responseBody, @Nullable Charset charset, @Nullable URI url, @Nullable HttpMethod method) {
private String getErrorMessage(int rawStatusCode, String statusText, @Nullable byte[] responseBody, @Nullable Charset charset,
@Nullable URI url, @Nullable HttpMethod method) {

String preface = getPreface(rawStatusCode, statusText, url, method);
StringBuilder msg = new StringBuilder(rawStatusCode + " " + statusText);
if (method != null) {
msg.append(" on ").append(method).append(" request");
}
if (url != null) {
msg.append(" for \"");
String urlString = url.toString();
int idx = urlString.indexOf('?');
if (idx != -1) {
msg.append(urlString, 0, idx);
}
else {
msg.append(urlString);
}
msg.append("\"");
}
msg.append(": ");
if (ObjectUtils.isEmpty(responseBody)) {
return preface + "[no body]";
msg.append("[no body]");
}

charset = (charset != null ? charset : StandardCharsets.UTF_8);

String bodyText = new String(responseBody, charset);
bodyText = LogFormatUtils.formatValue(bodyText, -1, true);

return preface + bodyText;
}

private String getPreface(int rawStatusCode, String statusText, @Nullable URI url, @Nullable HttpMethod method) {
StringBuilder preface = new StringBuilder(rawStatusCode + " " + statusText);
if (!ObjectUtils.isEmpty(method) && !ObjectUtils.isEmpty(url)) {
preface.append(" after ").append(method).append(" ").append(url).append(" ");
else {
charset = (charset != null ? charset : StandardCharsets.UTF_8);
String bodyText = new String(responseBody, charset);
bodyText = LogFormatUtils.formatValue(bodyText, -1, true);
msg.append(bodyText);
}
preface.append(": ");
return preface.toString();
}

/**
* Handle the error based on the resolved status code.
*
* <p>The default implementation delegates to
* {@link HttpClientErrorException#create} for errors in the 4xx range, to
* {@link HttpServerErrorException#create} for errors in the 5xx range,
* or otherwise raises {@link UnknownHttpStatusCodeException}.
* @since 5.0
* @see HttpClientErrorException#create
* @see HttpServerErrorException#create
*/
protected void handleError(ClientHttpResponse response, HttpStatusCode statusCode) throws IOException {
handleError(null, null, response, statusCode);
return msg.toString();
}

/**
Expand All @@ -213,12 +208,11 @@ protected void handleError(ClientHttpResponse response, HttpStatusCode statusCod
* {@link HttpClientErrorException#create} for errors in the 4xx range, to
* {@link HttpServerErrorException#create} for errors in the 5xx range,
* or otherwise raises {@link UnknownHttpStatusCodeException}.
* @since 5.0
* @since 6.2
* @see HttpClientErrorException#create
* @see HttpServerErrorException#create
*/
protected void handleError(@Nullable URI url, @Nullable HttpMethod method, ClientHttpResponse response,
HttpStatusCode statusCode) throws IOException {
protected void handleError(ClientHttpResponse response, HttpStatusCode statusCode, @Nullable URI url, @Nullable HttpMethod method) throws IOException {
String statusText = response.getStatusText();
HttpHeaders headers = response.getHeaders();
byte[] body = getResponseBody(response);
Expand Down
18 changes: 9 additions & 9 deletions spring-web/src/main/java/org/springframework/web/client/ExtractingResponseErrorHandler.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2024 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -138,7 +138,12 @@ protected boolean hasError(HttpStatusCode statusCode) {
}

@Override
public void handleError(ClientHttpResponse response, HttpStatusCode statusCode) throws IOException {
public void handleError(URI url, HttpMethod method, ClientHttpResponse response) throws IOException {
handleError(response, response.getStatusCode(), url, method);
}

@Override
protected void handleError(ClientHttpResponse response, HttpStatusCode statusCode, @Nullable URI url, @Nullable HttpMethod method) throws IOException {
if (this.statusMapping.containsKey(statusCode)) {
extract(this.statusMapping.get(statusCode), response);
}
Expand All @@ -147,17 +152,12 @@ public void handleError(ClientHttpResponse response, HttpStatusCode statusCode)
extract(this.seriesMapping.get(series), response);
}
else {
super.handleError(response, statusCode);
super.handleError(response, statusCode, url, method);
}
}

@Override
public void handleError(URI url, HttpMethod method, ClientHttpResponse response) throws IOException {
handleError(response, response.getStatusCode());
}

private void extract(@Nullable Class<? extends RestClientException> exceptionClass,
ClientHttpResponse response) throws IOException {
ClientHttpResponse response) throws IOException {

if (exceptionClass == null) {
return;
Expand Down
38 changes: 29 additions & 9 deletions ...ng-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java
View file
Expand Up @@ -29,6 +29,7 @@
import org.springframework.http.HttpStatusCode;
import org.springframework.http.MediaType;
import org.springframework.http.client.ClientHttpResponse;
import org.springframework.lang.Nullable;
import org.springframework.util.StreamUtils;

import static org.assertj.core.api.Assertions.assertThat;
Expand Down Expand Up @@ -80,19 +81,38 @@ void handleError() throws Exception {
}

@Test
public void handleErrorWithUrlAndMethod() throws Exception {
HttpHeaders headers = new HttpHeaders();
headers.setContentType(MediaType.TEXT_PLAIN);
void handleErrorWithUrlAndMethod() throws Exception {
setupClientHttpResponse(HttpStatus.NOT_FOUND, "Hello World");
assertThatExceptionOfType(HttpClientErrorException.class)
.isThrownBy(() -> handler.handleError(URI.create("https://example.com"), HttpMethod.GET, response))
.withMessage("404 Not Found on GET request for \"https://example.com\": \"Hello World\"");
}

given(response.getStatusCode()).willReturn(HttpStatus.NOT_FOUND);
given(response.getStatusText()).willReturn("Not Found");
given(response.getHeaders()).willReturn(headers);
given(response.getBody()).willReturn(new ByteArrayInputStream("Hello World".getBytes(StandardCharsets.UTF_8)));
@Test
void handleErrorWithUrlAndQueryParameters() throws Exception {
setupClientHttpResponse(HttpStatus.NOT_FOUND, "Hello World");
assertThatExceptionOfType(HttpClientErrorException.class)
.isThrownBy(() -> handler.handleError(URI.create("https://example.com/resource?access_token=123"), HttpMethod.GET, response))
.withMessage("404 Not Found on GET request for \"https://example.com/resource\": \"Hello World\"");
}

@Test
void handleErrorWithUrlAndNoBody() throws Exception {
setupClientHttpResponse(HttpStatus.NOT_FOUND, null);
assertThatExceptionOfType(HttpClientErrorException.class)
.isThrownBy(() -> handler.handleError(URI.create("https://example.com"), HttpMethod.GET, response))
.withMessage("404 Not Found after GET https://example.com : \"Hello World\"")
.satisfies(ex -> assertThat(ex.getResponseHeaders()).isSameAs(headers));
.withMessage("404 Not Found on GET request for \"https://example.com\": [no body]");
}

private void setupClientHttpResponse(HttpStatus status, @Nullable String textBody) throws Exception {
HttpHeaders headers = new HttpHeaders();
given(response.getStatusCode()).willReturn(status);
given(response.getStatusText()).willReturn(status.getReasonPhrase());
if (textBody != null) {
headers.setContentType(MediaType.TEXT_PLAIN);
given(response.getBody()).willReturn(new ByteArrayInputStream(textBody.getBytes(StandardCharsets.UTF_8)));
}
given(response.getHeaders()).willReturn(headers);
}

@Test
Expand Down
20 changes: 14 additions & 6 deletions spring-web/src/test/java/org/springframework/web/client/RestTemplateIntegrationTests.java
View file
Expand Up @@ -241,40 +241,48 @@ void patchForObject(ClientHttpRequestFactory clientHttpRequestFactory) {
void notFound(ClientHttpRequestFactory clientHttpRequestFactory) {
setUpClient(clientHttpRequestFactory);

String url = baseUrl + "/status/notfound";
assertThatExceptionOfType(HttpClientErrorException.class).isThrownBy(() ->
template.execute(baseUrl + "/status/notfound", HttpMethod.GET, null, null))
template.execute(url, HttpMethod.GET, null, null))
.satisfies(ex -> {
assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
assertThat(ex.getStatusText()).isNotNull();
assertThat(ex.getResponseBodyAsString()).isNotNull();
assertThat(ex.getMessage()).isEqualTo("404 Client Error after GET http://localhost:" + port + "/status/notfound : [no body]");
assertThat(ex.getMessage()).containsSubsequence("404", "on GET request for \"" + url + "\": [no body]");
assumeFalse(clientHttpRequestFactory instanceof JdkClientHttpRequestFactory, "JDK HttpClient does not expose status text");
assertThat(ex.getMessage()).isEqualTo("404 Client Error on GET request for \"" + url + "\": [no body]");
});
}

@ParameterizedRestTemplateTest
void badRequest(ClientHttpRequestFactory clientHttpRequestFactory) {
setUpClient(clientHttpRequestFactory);

String url = baseUrl + "/status/badrequest";
assertThatExceptionOfType(HttpClientErrorException.class).isThrownBy(() ->
template.execute(baseUrl + "/status/badrequest", HttpMethod.GET, null, null))
template.execute(url, HttpMethod.GET, null, null))
.satisfies(ex -> {
assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
assertThat(ex.getMessage()).containsSubsequence("400", "on GET request for \""+url+ "\": [no body]");
assumeFalse(clientHttpRequestFactory instanceof JdkClientHttpRequestFactory, "JDK HttpClient does not expose status text");
assertThat(ex.getMessage()).isEqualTo("400 Client Error after GET http://localhost:" + port + "/status/badrequest : [no body]");
assertThat(ex.getMessage()).isEqualTo("400 Client Error on GET request for \""+url+ "\": [no body]");
});
}

@ParameterizedRestTemplateTest
void serverError(ClientHttpRequestFactory clientHttpRequestFactory) {
setUpClient(clientHttpRequestFactory);

String url = baseUrl + "/status/server";
assertThatExceptionOfType(HttpServerErrorException.class).isThrownBy(() ->
template.execute(baseUrl + "/status/server", HttpMethod.GET, null, null))
template.execute(url, HttpMethod.GET, null, null))
.satisfies(ex -> {
assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
assertThat(ex.getStatusText()).isNotNull();
assertThat(ex.getResponseBodyAsString()).isNotNull();
assertThat(ex.getMessage()).isEqualTo("500 Server Error after GET http://localhost:" + port + "/status/server : [no body]");
assertThat(ex.getMessage()).containsSubsequence("500", "on GET request for \"" + url + "\": [no body]");
assumeFalse(clientHttpRequestFactory instanceof JdkClientHttpRequestFactory, "JDK HttpClient does not expose status text");
assertThat(ex.getMessage()).isEqualTo("500 Server Error on GET request for \"" + url + "\": [no body]");
});
}

Expand Down

0 comments on commit f85c4e1

Please sign in to comment.