Skip to content

Commit

Permalink
Polishing in DefaultResponseErrorHandler
Browse files Browse the repository at this point in the history 
See gh-34231
  • Loading branch information
@rstoyanchev @bclozel
rstoyanchev authored and bclozel committed Jan 10, 2025
1 parent 53b3b93 commit cdddf09
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 131 deletions.
140 changes: 66 additions & 74 deletions spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.CollectionUtils;
import org.springframework.util.FileCopyUtils;
import org.springframework.util.ObjectUtils;

/**
Expand Down Expand Up @@ -135,8 +134,7 @@ protected boolean hasError(int statusCode) {
*/
@Override
public void handleError(ClientHttpResponse response) throws IOException {
HttpStatusCode statusCode = response.getStatusCode();
handleError(response, statusCode, null, null);
handleError(response, response.getStatusCode(), null, null);
}

/**
Expand All @@ -159,46 +157,7 @@ public void handleError(ClientHttpResponse response) throws IOException {
*/
@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 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) {

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)) {
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();
handleError(response, response.getStatusCode(), url, method);
}

/**
Expand All @@ -211,7 +170,8 @@ private String getErrorMessage(int rawStatusCode, String statusText, @Nullable b
* @see HttpClientErrorException#create
* @see HttpServerErrorException#create
*/
protected void handleError(ClientHttpResponse response, HttpStatusCode statusCode,
protected void handleError(
ClientHttpResponse response, HttpStatusCode statusCode,
@Nullable URI url, @Nullable HttpMethod method) throws IOException {

String statusText = response.getStatusText();
Expand All @@ -238,6 +198,68 @@ else if (statusCode.is5xxServerError()) {
throw ex;
}

/**
* Read the body of the given response (for inclusion in a status exception).
* @param response the response to inspect
* @return the response body as a byte array,
* or an empty byte array if the body could not be read
* @since 4.3.8
*/
protected byte[] getResponseBody(ClientHttpResponse response) {
return RestClientUtils.getBody(response);
}

/**
* Determine the charset of the response (for inclusion in a status exception).
* @param response the response to inspect
* @return the associated charset, or {@code null} if none
* @since 4.3.8
*/
@Nullable
protected Charset getCharset(ClientHttpResponse response) {
MediaType contentType = response.getHeaders().getContentType();
return (contentType != null ? contentType.getCharset() : null);
}

/**
* Return an error message with details from the response body. For example:
* <pre>
* 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) {

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)) {
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();
}

/**
* Return a function for decoding the error content. This can be passed to
* {@link RestClientResponseException#setBodyConvertFunction(Function)}.
Expand Down Expand Up @@ -265,34 +287,4 @@ public InputStream getBody() {
};
}

/**
* Read the body of the given response (for inclusion in a status exception).
* @param response the response to inspect
* @return the response body as a byte array,
* or an empty byte array if the body could not be read
* @since 4.3.8
*/
protected byte[] getResponseBody(ClientHttpResponse response) {
try {
return FileCopyUtils.copyToByteArray(response.getBody());
}
catch (IOException ex) {
// ignore
}
return new byte[0];
}

/**
* Determine the charset of the response (for inclusion in a status exception).
* @param response the response to inspect
* @return the associated charset, or {@code null} if none
* @since 4.3.8
*/
@Nullable
protected Charset getCharset(ClientHttpResponse response) {
HttpHeaders headers = response.getHeaders();
MediaType contentType = headers.getContentType();
return (contentType != null ? contentType.getCharset() : null);
}

}
50 changes: 26 additions & 24 deletions spring-web/src/main/java/org/springframework/web/client/ExtractingResponseErrorHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,26 +32,27 @@
import org.springframework.util.CollectionUtils;

/**
* Implementation of {@link ResponseErrorHandler} that uses {@link HttpMessageConverter
* HttpMessageConverters} to convert HTTP error responses to {@link RestClientException
* RestClientExceptions}.
* Implementation of {@link ResponseErrorHandler} that uses
* {@link HttpMessageConverter HttpMessageConverters} to convert HTTP error
* responses to {@link RestClientException RestClientExceptions}.
*
* <p>To use this error handler, you must specify a
* {@linkplain #setStatusMapping(Map) status mapping} and/or a
* {@linkplain #setSeriesMapping(Map) series mapping}. If either of these mappings has a match
* for the {@linkplain ClientHttpResponse#getStatusCode() status code} of a given
* {@code ClientHttpResponse}, {@link #hasError(ClientHttpResponse)} will return
* {@code true}, and {@link #handleError(ClientHttpResponse)} will attempt to use the
* {@linkplain #setMessageConverters(List) configured message converters} to convert the response
* into the mapped subclass of {@link RestClientException}. Note that the
* {@linkplain #setStatusMapping(Map) status mapping} takes precedence over
* {@linkplain #setSeriesMapping(Map) series mapping}.
* {@linkplain #setSeriesMapping(Map) series mapping}. If either of these
* mappings has a match for the {@linkplain ClientHttpResponse#getStatusCode()
* status code} of a given {@code ClientHttpResponse},
* {@link #hasError(ClientHttpResponse)} will return {@code true}, and
* {@link #handleError(ClientHttpResponse)} will attempt to use the
* {@linkplain #setMessageConverters(List) configured message converters} to
* convert the response into the mapped subclass of {@link RestClientException}.
* Note that the {@linkplain #setStatusMapping(Map) status mapping} takes
* precedence over {@linkplain #setSeriesMapping(Map) series mapping}.
*
* <p>If there is no match, this error handler will default to the behavior of
* {@link DefaultResponseErrorHandler}. Note that you can override this default behavior
* by specifying a {@linkplain #setSeriesMapping(Map) series mapping} from
* {@code HttpStatus.Series#CLIENT_ERROR} and/or {@code HttpStatus.Series#SERVER_ERROR}
* to {@code null}.
* {@link DefaultResponseErrorHandler}. Note that you can override this default
* behavior by specifying a {@linkplain #setSeriesMapping(Map) series mapping}
* from {@code HttpStatus.Series#CLIENT_ERROR} and/or
* {@code HttpStatus.Series#SERVER_ERROR} to {@code null}.
*
* @author Simon Galperin
* @author Arjen Poutsma
Expand Down Expand Up @@ -126,27 +127,26 @@ public void setSeriesMapping(Map<HttpStatus.Series, Class<? extends RestClientEx
@Override
protected boolean hasError(HttpStatusCode statusCode) {
if (this.statusMapping.containsKey(statusCode)) {
return this.statusMapping.get(statusCode) != null;
return (this.statusMapping.get(statusCode) != null);
}
HttpStatus.Series series = HttpStatus.Series.resolve(statusCode.value());
if (this.seriesMapping.containsKey(series)) {
return this.seriesMapping.get(series) != null;
return (this.seriesMapping.get(series) != null);
}
else {
return super.hasError(statusCode);
}
}

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

@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);
}

HttpStatus.Series series = HttpStatus.Series.resolve(statusCode.value());
if (this.seriesMapping.containsKey(series)) {
extract(this.seriesMapping.get(series), response);
Expand All @@ -156,15 +156,17 @@ protected void handleError(ClientHttpResponse response, HttpStatusCode statusCod
}
}

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

if (exceptionClass == null) {
return;
}

HttpMessageConverterExtractor<? extends RestClientException> extractor =
new HttpMessageConverterExtractor<>(exceptionClass, this.messageConverters);

RestClientException exception = extractor.extractData(response);
if (exception != null) {
throw exception;
Expand Down
24 changes: 13 additions & 11 deletions ...ng-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.net.URI;
import java.nio.charset.StandardCharsets;

import org.junit.jupiter.api.Test;

Expand All @@ -32,6 +31,7 @@
import org.springframework.lang.Nullable;
import org.springframework.util.StreamUtils;

import static java.nio.charset.StandardCharsets.UTF_8;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.catchThrowable;
Expand Down Expand Up @@ -72,7 +72,7 @@ void handleError() throws Exception {
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)));
given(response.getBody()).willReturn(new ByteArrayInputStream("Hello World".getBytes(UTF_8)));

assertThatExceptionOfType(HttpClientErrorException.class)
.isThrownBy(() -> handler.handleError(response))
Expand All @@ -90,18 +90,20 @@ void handleErrorWithUrlAndMethod() throws Exception {

@Test
void handleErrorWithUrlAndQueryParameters() throws Exception {
String url = "https://example.com/resource";
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\"");
.isThrownBy(() -> handler.handleError(URI.create(url + "?access_token=123"), HttpMethod.GET, response))
.withMessage("404 Not Found on GET request for \"" + url + "\": \"Hello World\"");
}

@Test
void handleErrorWithUrlAndNoBody() throws Exception {
String url = "https://example.com";
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]");
.isThrownBy(() -> handler.handleError(URI.create(url), HttpMethod.GET, response))
.withMessage("404 Not Found on GET request for \"" + url + "\": [no body]");
}

private void setupClientHttpResponse(HttpStatus status, @Nullable String textBody) throws Exception {
Expand All @@ -110,7 +112,7 @@ private void setupClientHttpResponse(HttpStatus status, @Nullable String textBod
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.getBody()).willReturn(new ByteArrayInputStream(textBody.getBytes(UTF_8)));
}
given(response.getHeaders()).willReturn(headers);
}
Expand Down Expand Up @@ -187,7 +189,7 @@ void handleErrorForCustomClientError() throws Exception {
headers.setContentType(MediaType.TEXT_PLAIN);

String responseBody = "Hello World";
TestByteArrayInputStream body = new TestByteArrayInputStream(responseBody.getBytes(StandardCharsets.UTF_8));
TestByteArrayInputStream body = new TestByteArrayInputStream(responseBody.getBytes(UTF_8));

given(response.getStatusCode()).willReturn(statusCode);
given(response.getStatusText()).willReturn(statusText);
Expand Down Expand Up @@ -227,7 +229,7 @@ void handleErrorForCustomServerError() throws Exception {
headers.setContentType(MediaType.TEXT_PLAIN);

String responseBody = "Hello World";
TestByteArrayInputStream body = new TestByteArrayInputStream(responseBody.getBytes(StandardCharsets.UTF_8));
TestByteArrayInputStream body = new TestByteArrayInputStream(responseBody.getBytes(UTF_8));

given(response.getStatusCode()).willReturn(statusCode);
given(response.getStatusText()).willReturn(statusText);
Expand All @@ -250,7 +252,7 @@ void handleErrorForCustomServerError() throws Exception {
public void bodyAvailableAfterHasErrorForUnknownStatusCode() throws Exception {
HttpHeaders headers = new HttpHeaders();
headers.setContentType(MediaType.TEXT_PLAIN);
TestByteArrayInputStream body = new TestByteArrayInputStream("Hello World".getBytes(StandardCharsets.UTF_8));
TestByteArrayInputStream body = new TestByteArrayInputStream("Hello World".getBytes(UTF_8));

given(response.getStatusCode()).willReturn(HttpStatusCode.valueOf(999));
given(response.getStatusText()).willReturn("Custom status code");
Expand All @@ -259,7 +261,7 @@ public void bodyAvailableAfterHasErrorForUnknownStatusCode() throws Exception {

assertThat(handler.hasError(response)).isFalse();
assertThat(body.isClosed()).isFalse();
assertThat(StreamUtils.copyToString(response.getBody(), StandardCharsets.UTF_8)).isEqualTo("Hello World");
assertThat(StreamUtils.copyToString(response.getBody(), UTF_8)).isEqualTo("Hello World");
}


Expand Down
Loading

0 comments on commit cdddf09

Please sign in to comment.