Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use URL and HTTP method in DefaultResponseErrorHandler #28958

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -148,8 +150,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 @@ public void handleError() throws Exception {
.satisfies(ex -> assertThat(ex.getResponseHeaders()).isSameAs(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
public void handleErrorIOException() throws Exception {
HttpHeaders headers = new HttpHeaders();
Expand Down
Expand Up @@ -242,6 +242,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 @@ -253,7 +254,7 @@ void badRequest(ClientHttpRequestFactory clientHttpRequestFactory) {
template.execute(baseUrl + "/status/badrequest", HttpMethod.GET, null, null))
.satisfies(ex -> {
assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
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 @@ -267,6 +268,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