Skip to content

Commit a7ceedf

Browse files
snicollsnicoll
committed
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
2 parents 2350c8a + f85c4e1 commit a7ceedf

File tree

4 files changed

+122
-26
lines changed

4 files changed

+122
-26
lines changed

‎spring-web/src/main/java/org/springframework/web/client/DefaultResponseErrorHandler.java‎

Lines changed: 60 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,6 +19,7 @@
1919
import java.io.ByteArrayInputStream;
2020
import java.io.IOException;
2121
import java.io.InputStream;
22+
import java.net.URI;
2223
import java.nio.charset.Charset;
2324
import java.nio.charset.StandardCharsets;
2425
import java.util.Collections;
@@ -28,6 +29,7 @@
2829
import org.springframework.core.ResolvableType;
2930
import org.springframework.core.log.LogFormatUtils;
3031
import org.springframework.http.HttpHeaders;
32+
import org.springframework.http.HttpMethod;
3133
import org.springframework.http.HttpStatus;
3234
import org.springframework.http.HttpStatusCode;
3335
import org.springframework.http.MediaType;
@@ -129,35 +131,74 @@ protected boolean hasError(int statusCode) {
129131
* {@link HttpStatus} enum range.
130132
* </ul>
131133
* @throws UnknownHttpStatusCodeException in case of an unresolvable status code
132-
* @see #handleError(ClientHttpResponse, HttpStatusCode)
134+
* @see #handleError(ClientHttpResponse, HttpStatusCode, URI, HttpMethod)
133135
*/
134136
@Override
135137
public void handleError(ClientHttpResponse response) throws IOException {
136138
HttpStatusCode statusCode = response.getStatusCode();
137-
handleError(response, statusCode);
139+
handleError(response, statusCode, null, null);
140+
}
141+
142+
/**
143+
* Handle the error in the given response with the given resolved status code
144+
* and extra information providing access to the request URL and HTTP method.
145+
* <p>The default implementation throws:
146+
* <ul>
147+
* <li>{@link HttpClientErrorException} if the status code is in the 4xx
148+
* series, or one of its sub-classes such as
149+
* {@link HttpClientErrorException.BadRequest} and others.
150+
* <li>{@link HttpServerErrorException} if the status code is in the 5xx
151+
* series, or one of its sub-classes such as
152+
* {@link HttpServerErrorException.InternalServerError} and others.
153+
* <li>{@link UnknownHttpStatusCodeException} for error status codes not in the
154+
* {@link HttpStatus} enum range.
155+
* </ul>
156+
* @throws UnknownHttpStatusCodeException in case of an unresolvable status code
157+
* @since 6.2
158+
* @see #handleError(ClientHttpResponse, HttpStatusCode, URI, HttpMethod)
159+
*/
160+
@Override
161+
public void handleError(URI url, HttpMethod method, ClientHttpResponse response) throws IOException {
162+
HttpStatusCode statusCode = response.getStatusCode();
163+
handleError(response, statusCode, url, method);
138164
}
139165

140166
/**
141167
* Return error message with details from the response body. For example:
142168
* <pre>
143-
* 404 Not Found: [{'id': 123, 'message': 'my message'}]
169+
* 404 Not Found on GET request for "https://example.com": [{'id': 123, 'message': 'my message'}]
144170
* </pre>
145171
*/
146-
private String getErrorMessage(
147-
int rawStatusCode, String statusText, @Nullable byte[] responseBody, @Nullable Charset charset) {
148-
149-
String preface = rawStatusCode + " " + statusText + ": ";
172+
private String getErrorMessage(int rawStatusCode, String statusText, @Nullable byte[] responseBody, @Nullable Charset charset,
173+
@Nullable URI url, @Nullable HttpMethod method) {
150174

175+
StringBuilder msg = new StringBuilder(rawStatusCode + " " + statusText);
176+
if (method != null) {
177+
msg.append(" on ").append(method).append(" request");
178+
}
179+
if (url != null) {
180+
msg.append(" for \"");
181+
String urlString = url.toString();
182+
int idx = urlString.indexOf('?');
183+
if (idx != -1) {
184+
msg.append(urlString, 0, idx);
185+
}
186+
else {
187+
msg.append(urlString);
188+
}
189+
msg.append("\"");
190+
}
191+
msg.append(": ");
151192
if (ObjectUtils.isEmpty(responseBody)) {
152-
return preface + "[no body]";
193+
msg.append("[no body]");
153194
}
154-
155-
charset = (charset != null ? charset : StandardCharsets.UTF_8);
156-
157-
String bodyText = new String(responseBody, charset);
158-
bodyText = LogFormatUtils.formatValue(bodyText, -1, true);
159-
160-
return preface + bodyText;
195+
else {
196+
charset = (charset != null ? charset : StandardCharsets.UTF_8);
197+
String bodyText = new String(responseBody, charset);
198+
bodyText = LogFormatUtils.formatValue(bodyText, -1, true);
199+
msg.append(bodyText);
200+
}
201+
return msg.toString();
161202
}
162203

163204
/**
@@ -167,16 +208,16 @@ private String getErrorMessage(
167208
* {@link HttpClientErrorException#create} for errors in the 4xx range, to
168209
* {@link HttpServerErrorException#create} for errors in the 5xx range,
169210
* or otherwise raises {@link UnknownHttpStatusCodeException}.
170-
* @since 5.0
211+
* @since 6.2
171212
* @see HttpClientErrorException#create
172213
* @see HttpServerErrorException#create
173214
*/
174-
protected void handleError(ClientHttpResponse response, HttpStatusCode statusCode) throws IOException {
215+
protected void handleError(ClientHttpResponse response, HttpStatusCode statusCode, @Nullable URI url, @Nullable HttpMethod method) throws IOException {
175216
String statusText = response.getStatusText();
176217
HttpHeaders headers = response.getHeaders();
177218
byte[] body = getResponseBody(response);
178219
Charset charset = getCharset(response);
179-
String message = getErrorMessage(statusCode.value(), statusText, body, charset);
220+
String message = getErrorMessage(statusCode.value(), statusText, body, charset, url, method);
180221

181222
RestClientResponseException ex;
182223
if (statusCode.is4xxClientError()) {

‎spring-web/src/main/java/org/springframework/web/client/ExtractingResponseErrorHandler.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2024 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -17,11 +17,13 @@
1717
package org.springframework.web.client;
1818

1919
import java.io.IOException;
20+
import java.net.URI;
2021
import java.util.Collections;
2122
import java.util.LinkedHashMap;
2223
import java.util.List;
2324
import java.util.Map;
2425

26+
import org.springframework.http.HttpMethod;
2527
import org.springframework.http.HttpStatus;
2628
import org.springframework.http.HttpStatusCode;
2729
import org.springframework.http.client.ClientHttpResponse;
@@ -136,7 +138,12 @@ protected boolean hasError(HttpStatusCode statusCode) {
136138
}
137139

138140
@Override
139-
public void handleError(ClientHttpResponse response, HttpStatusCode statusCode) throws IOException {
141+
public void handleError(URI url, HttpMethod method, ClientHttpResponse response) throws IOException {
142+
handleError(response, response.getStatusCode(), url, method);
143+
}
144+
145+
@Override
146+
protected void handleError(ClientHttpResponse response, HttpStatusCode statusCode, @Nullable URI url, @Nullable HttpMethod method) throws IOException {
140147
if (this.statusMapping.containsKey(statusCode)) {
141148
extract(this.statusMapping.get(statusCode), response);
142149
}
@@ -145,7 +152,7 @@ public void handleError(ClientHttpResponse response, HttpStatusCode statusCode)
145152
extract(this.seriesMapping.get(series), response);
146153
}
147154
else {
148-
super.handleError(response, statusCode);
155+
super.handleError(response, statusCode, url, method);
149156
}
150157
}
151158

‎spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerTests.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,18 @@
1818

1919
import java.io.ByteArrayInputStream;
2020
import java.io.IOException;
21+
import java.net.URI;
2122
import java.nio.charset.StandardCharsets;
2223

2324
import org.junit.jupiter.api.Test;
2425

2526
import org.springframework.http.HttpHeaders;
27+
import org.springframework.http.HttpMethod;
2628
import org.springframework.http.HttpStatus;
2729
import org.springframework.http.HttpStatusCode;
2830
import org.springframework.http.MediaType;
2931
import org.springframework.http.client.ClientHttpResponse;
32+
import org.springframework.lang.Nullable;
3033
import org.springframework.util.StreamUtils;
3134

3235
import static org.assertj.core.api.Assertions.assertThat;
@@ -77,6 +80,41 @@ void handleError() throws Exception {
7780
.satisfies(ex -> assertThat(ex.getResponseHeaders()).isEqualTo(headers));
7881
}
7982

83+
@Test
84+
void handleErrorWithUrlAndMethod() throws Exception {
85+
setupClientHttpResponse(HttpStatus.NOT_FOUND, "Hello World");
86+
assertThatExceptionOfType(HttpClientErrorException.class)
87+
.isThrownBy(() -> handler.handleError(URI.create("https://example.com"), HttpMethod.GET, response))
88+
.withMessage("404 Not Found on GET request for \"https://example.com\": \"Hello World\"");
89+
}
90+
91+
@Test
92+
void handleErrorWithUrlAndQueryParameters() throws Exception {
93+
setupClientHttpResponse(HttpStatus.NOT_FOUND, "Hello World");
94+
assertThatExceptionOfType(HttpClientErrorException.class)
95+
.isThrownBy(() -> handler.handleError(URI.create("https://example.com/resource?access_token=123"), HttpMethod.GET, response))
96+
.withMessage("404 Not Found on GET request for \"https://example.com/resource\": \"Hello World\"");
97+
}
98+
99+
@Test
100+
void handleErrorWithUrlAndNoBody() throws Exception {
101+
setupClientHttpResponse(HttpStatus.NOT_FOUND, null);
102+
assertThatExceptionOfType(HttpClientErrorException.class)
103+
.isThrownBy(() -> handler.handleError(URI.create("https://example.com"), HttpMethod.GET, response))
104+
.withMessage("404 Not Found on GET request for \"https://example.com\": [no body]");
105+
}
106+
107+
private void setupClientHttpResponse(HttpStatus status, @Nullable String textBody) throws Exception {
108+
HttpHeaders headers = new HttpHeaders();
109+
given(response.getStatusCode()).willReturn(status);
110+
given(response.getStatusText()).willReturn(status.getReasonPhrase());
111+
if (textBody != null) {
112+
headers.setContentType(MediaType.TEXT_PLAIN);
113+
given(response.getBody()).willReturn(new ByteArrayInputStream(textBody.getBytes(StandardCharsets.UTF_8)));
114+
}
115+
given(response.getHeaders()).willReturn(headers);
116+
}
117+
80118
@Test
81119
void handleErrorIOException() throws Exception {
82120
HttpHeaders headers = new HttpHeaders();

‎spring-web/src/test/java/org/springframework/web/client/RestTemplateIntegrationTests.java‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,38 +241,48 @@ void patchForObject(ClientHttpRequestFactory clientHttpRequestFactory) {
241241
void notFound(ClientHttpRequestFactory clientHttpRequestFactory) {
242242
setUpClient(clientHttpRequestFactory);
243243

244+
String url = baseUrl + "/status/notfound";
244245
assertThatExceptionOfType(HttpClientErrorException.class).isThrownBy(() ->
245-
template.execute(baseUrl + "/status/notfound", HttpMethod.GET, null, null))
246+
template.execute(url, HttpMethod.GET, null, null))
246247
.satisfies(ex -> {
247248
assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
248249
assertThat(ex.getStatusText()).isNotNull();
249250
assertThat(ex.getResponseBodyAsString()).isNotNull();
251+
assertThat(ex.getMessage()).containsSubsequence("404", "on GET request for \"" + url + "\": [no body]");
252+
assumeFalse(clientHttpRequestFactory instanceof JdkClientHttpRequestFactory, "JDK HttpClient does not expose status text");
253+
assertThat(ex.getMessage()).isEqualTo("404 Client Error on GET request for \"" + url + "\": [no body]");
250254
});
251255
}
252256

253257
@ParameterizedRestTemplateTest
254258
void badRequest(ClientHttpRequestFactory clientHttpRequestFactory) {
255259
setUpClient(clientHttpRequestFactory);
256260

261+
String url = baseUrl + "/status/badrequest";
257262
assertThatExceptionOfType(HttpClientErrorException.class).isThrownBy(() ->
258-
template.execute(baseUrl + "/status/badrequest", HttpMethod.GET, null, null))
263+
template.execute(url, HttpMethod.GET, null, null))
259264
.satisfies(ex -> {
260265
assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.BAD_REQUEST);
266+
assertThat(ex.getMessage()).containsSubsequence("400", "on GET request for \""+url+ "\": [no body]");
261267
assumeFalse(clientHttpRequestFactory instanceof JdkClientHttpRequestFactory, "JDK HttpClient does not expose status text");
262-
assertThat(ex.getMessage()).isEqualTo("400 Client Error: [no body]");
268+
assertThat(ex.getMessage()).isEqualTo("400 Client Error on GET request for \""+url+ "\": [no body]");
263269
});
264270
}
265271

266272
@ParameterizedRestTemplateTest
267273
void serverError(ClientHttpRequestFactory clientHttpRequestFactory) {
268274
setUpClient(clientHttpRequestFactory);
269275

276+
String url = baseUrl + "/status/server";
270277
assertThatExceptionOfType(HttpServerErrorException.class).isThrownBy(() ->
271-
template.execute(baseUrl + "/status/server", HttpMethod.GET, null, null))
278+
template.execute(url, HttpMethod.GET, null, null))
272279
.satisfies(ex -> {
273280
assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.INTERNAL_SERVER_ERROR);
274281
assertThat(ex.getStatusText()).isNotNull();
275282
assertThat(ex.getResponseBodyAsString()).isNotNull();
283+
assertThat(ex.getMessage()).containsSubsequence("500", "on GET request for \"" + url + "\": [no body]");
284+
assumeFalse(clientHttpRequestFactory instanceof JdkClientHttpRequestFactory, "JDK HttpClient does not expose status text");
285+
assertThat(ex.getMessage()).isEqualTo("500 Server Error on GET request for \"" + url + "\": [no body]");
276286
});
277287
}
278288

0 commit comments

Comments
 (0)