Merge pull request #28958 from jerzykrlk
* pr/28958: Polish "Include URL and HTTP method in DefaultResponseErrorHandler" Include URL and HTTP method in DefaultResponseErrorHandler Closes gh-28958
This commit is contained in:
commit
a7ceedf2bb
|
@ -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.
|
||||
|
@ -19,6 +19,7 @@ package org.springframework.web.client;
|
|||
import java.io.ByteArrayInputStream;
|
||||
import java.io.IOException;
|
||||
import java.io.InputStream;
|
||||
import java.net.URI;
|
||||
import java.nio.charset.Charset;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.util.Collections;
|
||||
|
@ -28,6 +29,7 @@ import java.util.function.Function;
|
|||
import org.springframework.core.ResolvableType;
|
||||
import org.springframework.core.log.LogFormatUtils;
|
||||
import org.springframework.http.HttpHeaders;
|
||||
import org.springframework.http.HttpMethod;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.HttpStatusCode;
|
||||
import org.springframework.http.MediaType;
|
||||
|
@ -129,35 +131,74 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
|
|||
* {@link HttpStatus} enum range.
|
||||
* </ul>
|
||||
* @throws UnknownHttpStatusCodeException in case of an unresolvable status code
|
||||
* @see #handleError(ClientHttpResponse, HttpStatusCode)
|
||||
* @see #handleError(ClientHttpResponse, HttpStatusCode, URI, HttpMethod)
|
||||
*/
|
||||
@Override
|
||||
public void handleError(ClientHttpResponse response) throws IOException {
|
||||
HttpStatusCode statusCode = response.getStatusCode();
|
||||
handleError(response, statusCode);
|
||||
handleError(response, statusCode, null, null);
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
* series, or one of its sub-classes such as
|
||||
* {@link HttpClientErrorException.BadRequest} and others.
|
||||
* <li>{@link HttpServerErrorException} if the status code is in the 5xx
|
||||
* series, or one of its sub-classes such as
|
||||
* {@link HttpServerErrorException.InternalServerError} and others.
|
||||
* <li>{@link UnknownHttpStatusCodeException} for error status codes not in the
|
||||
* {@link HttpStatus} enum range.
|
||||
* </ul>
|
||||
* @throws UnknownHttpStatusCodeException in case of an unresolvable status code
|
||||
* @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(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) {
|
||||
private String getErrorMessage(int rawStatusCode, String statusText, @Nullable byte[] responseBody, @Nullable Charset charset,
|
||||
@Nullable URI url, @Nullable HttpMethod method) {
|
||||
|
||||
String preface = rawStatusCode + " " + statusText + ": ";
|
||||
|
||||
if (ObjectUtils.isEmpty(responseBody)) {
|
||||
return preface + "[no body]";
|
||||
StringBuilder msg = new StringBuilder(rawStatusCode + " " + statusText);
|
||||
if (method != null) {
|
||||
msg.append(" on ").append(method).append(" request");
|
||||
}
|
||||
|
||||
charset = (charset != null ? charset : StandardCharsets.UTF_8);
|
||||
|
||||
String bodyText = new String(responseBody, charset);
|
||||
bodyText = LogFormatUtils.formatValue(bodyText, -1, true);
|
||||
|
||||
return preface + bodyText;
|
||||
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)) {
|
||||
msg.append("[no body]");
|
||||
}
|
||||
else {
|
||||
charset = (charset != null ? charset : StandardCharsets.UTF_8);
|
||||
String bodyText = new String(responseBody, charset);
|
||||
bodyText = LogFormatUtils.formatValue(bodyText, -1, true);
|
||||
msg.append(bodyText);
|
||||
}
|
||||
return msg.toString();
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -167,16 +208,16 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
|
|||
* {@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(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);
|
||||
Charset charset = getCharset(response);
|
||||
String message = getErrorMessage(statusCode.value(), statusText, body, charset);
|
||||
String message = getErrorMessage(statusCode.value(), statusText, body, charset, url, method);
|
||||
|
||||
RestClientResponseException ex;
|
||||
if (statusCode.is4xxClientError()) {
|
||||
|
|
|
@ -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.
|
||||
|
@ -17,11 +17,13 @@
|
|||
package org.springframework.web.client;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.net.URI;
|
||||
import java.util.Collections;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
import org.springframework.http.HttpMethod;
|
||||
import org.springframework.http.HttpStatus;
|
||||
import org.springframework.http.HttpStatusCode;
|
||||
import org.springframework.http.client.ClientHttpResponse;
|
||||
|
@ -136,7 +138,12 @@ public class ExtractingResponseErrorHandler extends DefaultResponseErrorHandler
|
|||
}
|
||||
|
||||
@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);
|
||||
}
|
||||
|
@ -145,7 +152,7 @@ public class ExtractingResponseErrorHandler extends DefaultResponseErrorHandler
|
|||
extract(this.seriesMapping.get(series), response);
|
||||
}
|
||||
else {
|
||||
super.handleError(response, statusCode);
|
||||
super.handleError(response, statusCode, url, method);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -18,15 +18,18 @@ package org.springframework.web.client;
|
|||
|
||||
import java.io.ByteArrayInputStream;
|
||||
import java.io.IOException;
|
||||
import java.net.URI;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import org.springframework.http.HttpHeaders;
|
||||
import org.springframework.http.HttpMethod;
|
||||
import org.springframework.http.HttpStatus;
|
||||
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;
|
||||
|
@ -77,6 +80,41 @@ class DefaultResponseErrorHandlerTests {
|
|||
.satisfies(ex -> assertThat(ex.getResponseHeaders()).isEqualTo(headers));
|
||||
}
|
||||
|
||||
@Test
|
||||
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\"");
|
||||
}
|
||||
|
||||
@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 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
|
||||
void handleErrorIOException() throws Exception {
|
||||
HttpHeaders headers = new HttpHeaders();
|
||||
|
|
|
@ -241,12 +241,16 @@ class RestTemplateIntegrationTests extends AbstractMockWebServerTests {
|
|||
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()).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]");
|
||||
});
|
||||
}
|
||||
|
||||
|
@ -254,12 +258,14 @@ class RestTemplateIntegrationTests extends AbstractMockWebServerTests {
|
|||
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: [no body]");
|
||||
assertThat(ex.getMessage()).isEqualTo("400 Client Error on GET request for \""+url+ "\": [no body]");
|
||||
});
|
||||
}
|
||||
|
||||
|
@ -267,12 +273,16 @@ class RestTemplateIntegrationTests extends AbstractMockWebServerTests {
|
|||
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()).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]");
|
||||
});
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue