Skip to content

Commit

Permalink
Include URL and HTTP method in DefaultResponseErrorHandler
Browse files Browse the repository at this point in the history
  • Loading branch information
jerzykrlk authored and snicoll committed Apr 30, 2024
1 parent 2350c8a commit 4972d18
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 8 deletions.
Expand Up @@ -19,6 +19,7 @@
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;
Expand All @@ -28,6 +29,7 @@
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;
Expand Down Expand Up @@ -129,12 +131,33 @@ protected boolean hasError(int statusCode) {
* {@link HttpStatus} enum range.
* </ul>
* @throws UnknownHttpStatusCodeException in case of an unresolvable status code
* @see #handleError(ClientHttpResponse, HttpStatusCode)
* @see #handleError(URI, HttpMethod, ClientHttpResponse, HttpStatusCode)
*/
@Override
public void handleError(ClientHttpResponse response) throws IOException {
handleError(null, null, response);
}

/**
* Handle the error in the given response with the given resolved status code.
* <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
* @see #handleError(URI, HttpMethod, ClientHttpResponse, HttpStatusCode)
*/
@Override
public void handleError(URI url, HttpMethod method, ClientHttpResponse response) throws IOException {
HttpStatusCode statusCode = response.getStatusCode();
handleError(response, statusCode);
handleError(url, method, response, statusCode);
}

/**
Expand All @@ -144,10 +167,9 @@ public void handleError(ClientHttpResponse response) throws IOException {
* </pre>
*/
private String getErrorMessage(
int rawStatusCode, String statusText, @Nullable byte[] responseBody, @Nullable Charset charset) {

String preface = rawStatusCode + " " + statusText + ": ";
int rawStatusCode, String statusText, @Nullable byte[] responseBody, @Nullable Charset charset, @Nullable URI url, @Nullable HttpMethod method) {

String preface = getPreface(rawStatusCode, statusText, url, method);
if (ObjectUtils.isEmpty(responseBody)) {
return preface + "[no body]";
}
Expand All @@ -160,6 +182,15 @@ private String getErrorMessage(
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(" ");
}
preface.append(": ");
return preface.toString();
}

/**
* Handle the error based on the resolved status code.
*
Expand All @@ -172,11 +203,27 @@ private String getErrorMessage(
* @see HttpServerErrorException#create
*/
protected void handleError(ClientHttpResponse response, HttpStatusCode statusCode) throws IOException {
handleError(null, null, response, statusCode);
}

/**
* 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(@Nullable URI url, @Nullable HttpMethod method, ClientHttpResponse response,
HttpStatusCode statusCode) 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()) {
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -149,8 +151,13 @@ public void handleError(ClientHttpResponse response, HttpStatusCode statusCode)
}
}

@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
Expand Up @@ -18,11 +18,13 @@

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;
Expand Down Expand Up @@ -77,6 +79,22 @@ void handleError() throws Exception {
.satisfies(ex -> assertThat(ex.getResponseHeaders()).isEqualTo(headers));
}

@Test
public void handleErrorWithUrlAndMethod() throws Exception {
HttpHeaders headers = new HttpHeaders();
headers.setContentType(MediaType.TEXT_PLAIN);

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

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

@Test
void handleErrorIOException() throws Exception {
HttpHeaders headers = new HttpHeaders();
Expand Down
Expand Up @@ -247,6 +247,7 @@ void notFound(ClientHttpRequestFactory clientHttpRequestFactory) {
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]");
});
}

Expand All @@ -259,7 +260,7 @@ void badRequest(ClientHttpRequestFactory clientHttpRequestFactory) {
.satisfies(ex -> {
assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
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 after GET http://localhost:" + port + "/status/badrequest : [no body]");
});
}

Expand All @@ -273,6 +274,7 @@ void serverError(ClientHttpRequestFactory clientHttpRequestFactory) {
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]");
});
}

Expand Down

0 comments on commit 4972d18

Please sign in to comment.