Skip to content

Commit a0cc641

Browse files
rstoyanchevrstoyanchev
committed
Polishing in DefaultResponseErrorHandler
See gh-33980
1 parent 5fa9460 commit a0cc641

File tree

4 files changed

+127
-131
lines changed

4 files changed

+127
-131
lines changed

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

+66-74
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
import org.springframework.lang.Nullable;
3939
import org.springframework.util.Assert;
4040
import org.springframework.util.CollectionUtils;
41-
import org.springframework.util.FileCopyUtils;
4241
import org.springframework.util.ObjectUtils;
4342

4443
/**
@@ -135,8 +134,7 @@ protected boolean hasError(int statusCode) {
135134
*/
136135
@Override
137136
public void handleError(ClientHttpResponse response) throws IOException {
138-
HttpStatusCode statusCode = response.getStatusCode();
139-
handleError(response, statusCode, null, null);
137+
handleError(response, response.getStatusCode(), null, null);
140138
}
141139

142140
/**
@@ -159,46 +157,7 @@ public void handleError(ClientHttpResponse response) throws IOException {
159157
*/
160158
@Override
161159
public void handleError(URI url, HttpMethod method, ClientHttpResponse response) throws IOException {
162-
HttpStatusCode statusCode = response.getStatusCode();
163-
handleError(response, statusCode, url, method);
164-
}
165-
166-
/**
167-
* Return error message with details from the response body. For example:
168-
* <pre>
169-
* 404 Not Found on GET request for "https://example.com": [{'id': 123, 'message': 'my message'}]
170-
* </pre>
171-
*/
172-
private String getErrorMessage(int rawStatusCode, String statusText, @Nullable byte[] responseBody, @Nullable Charset charset,
173-
@Nullable URI url, @Nullable HttpMethod method) {
174-
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(": ");
192-
if (ObjectUtils.isEmpty(responseBody)) {
193-
msg.append("[no body]");
194-
}
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();
160+
handleError(response, response.getStatusCode(), url, method);
202161
}
203162

204163
/**
@@ -211,7 +170,8 @@ private String getErrorMessage(int rawStatusCode, String statusText, @Nullable b
211170
* @see HttpClientErrorException#create
212171
* @see HttpServerErrorException#create
213172
*/
214-
protected void handleError(ClientHttpResponse response, HttpStatusCode statusCode,
173+
protected void handleError(
174+
ClientHttpResponse response, HttpStatusCode statusCode,
215175
@Nullable URI url, @Nullable HttpMethod method) throws IOException {
216176

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

201+
/**
202+
* Read the body of the given response (for inclusion in a status exception).
203+
* @param response the response to inspect
204+
* @return the response body as a byte array,
205+
* or an empty byte array if the body could not be read
206+
* @since 4.3.8
207+
*/
208+
protected byte[] getResponseBody(ClientHttpResponse response) {
209+
return RestClientUtils.getBody(response);
210+
}
211+
212+
/**
213+
* Determine the charset of the response (for inclusion in a status exception).
214+
* @param response the response to inspect
215+
* @return the associated charset, or {@code null} if none
216+
* @since 4.3.8
217+
*/
218+
@Nullable
219+
protected Charset getCharset(ClientHttpResponse response) {
220+
MediaType contentType = response.getHeaders().getContentType();
221+
return (contentType != null ? contentType.getCharset() : null);
222+
}
223+
224+
/**
225+
* Return an error message with details from the response body. For example:
226+
* <pre>
227+
* 404 Not Found on GET request for "https://example.com": [{'id': 123, 'message': 'my message'}]
228+
* </pre>
229+
*/
230+
private String getErrorMessage(
231+
int rawStatusCode, String statusText, @Nullable byte[] responseBody, @Nullable Charset charset,
232+
@Nullable URI url, @Nullable HttpMethod method) {
233+
234+
StringBuilder msg = new StringBuilder(rawStatusCode + " " + statusText);
235+
if (method != null) {
236+
msg.append(" on ").append(method).append(" request");
237+
}
238+
if (url != null) {
239+
msg.append(" for \"");
240+
String urlString = url.toString();
241+
int idx = urlString.indexOf('?');
242+
if (idx != -1) {
243+
msg.append(urlString, 0, idx);
244+
}
245+
else {
246+
msg.append(urlString);
247+
}
248+
msg.append("\"");
249+
}
250+
msg.append(": ");
251+
if (ObjectUtils.isEmpty(responseBody)) {
252+
msg.append("[no body]");
253+
}
254+
else {
255+
charset = (charset != null ? charset : StandardCharsets.UTF_8);
256+
String bodyText = new String(responseBody, charset);
257+
bodyText = LogFormatUtils.formatValue(bodyText, -1, true);
258+
msg.append(bodyText);
259+
}
260+
return msg.toString();
261+
}
262+
241263
/**
242264
* Return a function for decoding the error content. This can be passed to
243265
* {@link RestClientResponseException#setBodyConvertFunction(Function)}.
@@ -265,34 +287,4 @@ public InputStream getBody() {
265287
};
266288
}
267289

268-
/**
269-
* Read the body of the given response (for inclusion in a status exception).
270-
* @param response the response to inspect
271-
* @return the response body as a byte array,
272-
* or an empty byte array if the body could not be read
273-
* @since 4.3.8
274-
*/
275-
protected byte[] getResponseBody(ClientHttpResponse response) {
276-
try {
277-
return FileCopyUtils.copyToByteArray(response.getBody());
278-
}
279-
catch (IOException ex) {
280-
// ignore
281-
}
282-
return new byte[0];
283-
}
284-
285-
/**
286-
* Determine the charset of the response (for inclusion in a status exception).
287-
* @param response the response to inspect
288-
* @return the associated charset, or {@code null} if none
289-
* @since 4.3.8
290-
*/
291-
@Nullable
292-
protected Charset getCharset(ClientHttpResponse response) {
293-
HttpHeaders headers = response.getHeaders();
294-
MediaType contentType = headers.getContentType();
295-
return (contentType != null ? contentType.getCharset() : null);
296-
}
297-
298290
}

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

+26-24
Original file line numberDiff line numberDiff line change
@@ -32,26 +32,27 @@
3232
import org.springframework.util.CollectionUtils;
3333

3434
/**
35-
* Implementation of {@link ResponseErrorHandler} that uses {@link HttpMessageConverter
36-
* HttpMessageConverters} to convert HTTP error responses to {@link RestClientException
37-
* RestClientExceptions}.
35+
* Implementation of {@link ResponseErrorHandler} that uses
36+
* {@link HttpMessageConverter HttpMessageConverters} to convert HTTP error
37+
* responses to {@link RestClientException RestClientExceptions}.
3838
*
3939
* <p>To use this error handler, you must specify a
4040
* {@linkplain #setStatusMapping(Map) status mapping} and/or a
41-
* {@linkplain #setSeriesMapping(Map) series mapping}. If either of these mappings has a match
42-
* for the {@linkplain ClientHttpResponse#getStatusCode() status code} of a given
43-
* {@code ClientHttpResponse}, {@link #hasError(ClientHttpResponse)} will return
44-
* {@code true}, and {@link #handleError(ClientHttpResponse)} will attempt to use the
45-
* {@linkplain #setMessageConverters(List) configured message converters} to convert the response
46-
* into the mapped subclass of {@link RestClientException}. Note that the
47-
* {@linkplain #setStatusMapping(Map) status mapping} takes precedence over
48-
* {@linkplain #setSeriesMapping(Map) series mapping}.
41+
* {@linkplain #setSeriesMapping(Map) series mapping}. If either of these
42+
* mappings has a match for the {@linkplain ClientHttpResponse#getStatusCode()
43+
* status code} of a given {@code ClientHttpResponse},
44+
* {@link #hasError(ClientHttpResponse)} will return {@code true}, and
45+
* {@link #handleError(ClientHttpResponse)} will attempt to use the
46+
* {@linkplain #setMessageConverters(List) configured message converters} to
47+
* convert the response into the mapped subclass of {@link RestClientException}.
48+
* Note that the {@linkplain #setStatusMapping(Map) status mapping} takes
49+
* precedence over {@linkplain #setSeriesMapping(Map) series mapping}.
4950
*
5051
* <p>If there is no match, this error handler will default to the behavior of
51-
* {@link DefaultResponseErrorHandler}. Note that you can override this default behavior
52-
* by specifying a {@linkplain #setSeriesMapping(Map) series mapping} from
53-
* {@code HttpStatus.Series#CLIENT_ERROR} and/or {@code HttpStatus.Series#SERVER_ERROR}
54-
* to {@code null}.
52+
* {@link DefaultResponseErrorHandler}. Note that you can override this default
53+
* behavior by specifying a {@linkplain #setSeriesMapping(Map) series mapping}
54+
* from {@code HttpStatus.Series#CLIENT_ERROR} and/or
55+
* {@code HttpStatus.Series#SERVER_ERROR} to {@code null}.
5556
*
5657
* @author Simon Galperin
5758
* @author Arjen Poutsma
@@ -126,27 +127,26 @@ public void setSeriesMapping(Map<HttpStatus.Series, Class<? extends RestClientEx
126127
@Override
127128
protected boolean hasError(HttpStatusCode statusCode) {
128129
if (this.statusMapping.containsKey(statusCode)) {
129-
return this.statusMapping.get(statusCode) != null;
130+
return (this.statusMapping.get(statusCode) != null);
130131
}
131132
HttpStatus.Series series = HttpStatus.Series.resolve(statusCode.value());
132133
if (this.seriesMapping.containsKey(series)) {
133-
return this.seriesMapping.get(series) != null;
134+
return (this.seriesMapping.get(series) != null);
134135
}
135136
else {
136137
return super.hasError(statusCode);
137138
}
138139
}
139140

140141
@Override
141-
public void handleError(URI url, HttpMethod method, ClientHttpResponse response) throws IOException {
142-
handleError(response, response.getStatusCode(), url, method);
143-
}
142+
protected void handleError(
143+
ClientHttpResponse response, HttpStatusCode statusCode,
144+
@Nullable URI url, @Nullable HttpMethod method) throws IOException {
144145

145-
@Override
146-
protected void handleError(ClientHttpResponse response, HttpStatusCode statusCode, @Nullable URI url, @Nullable HttpMethod method) throws IOException {
147146
if (this.statusMapping.containsKey(statusCode)) {
148147
extract(this.statusMapping.get(statusCode), response);
149148
}
149+
150150
HttpStatus.Series series = HttpStatus.Series.resolve(statusCode.value());
151151
if (this.seriesMapping.containsKey(series)) {
152152
extract(this.seriesMapping.get(series), response);
@@ -156,15 +156,17 @@ protected void handleError(ClientHttpResponse response, HttpStatusCode statusCod
156156
}
157157
}
158158

159-
private void extract(@Nullable Class<? extends RestClientException> exceptionClass,
160-
ClientHttpResponse response) throws IOException {
159+
private void extract(
160+
@Nullable Class<? extends RestClientException> exceptionClass, ClientHttpResponse response)
161+
throws IOException {
161162

162163
if (exceptionClass == null) {
163164
return;
164165
}
165166

166167
HttpMessageConverterExtractor<? extends RestClientException> extractor =
167168
new HttpMessageConverterExtractor<>(exceptionClass, this.messageConverters);
169+
168170
RestClientException exception = extractor.extractData(response);
169171
if (exception != null) {
170172
throw exception;

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

+13-11
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import java.io.ByteArrayInputStream;
2020
import java.io.IOException;
2121
import java.net.URI;
22-
import java.nio.charset.StandardCharsets;
2322

2423
import org.junit.jupiter.api.Test;
2524

@@ -32,6 +31,7 @@
3231
import org.springframework.lang.Nullable;
3332
import org.springframework.util.StreamUtils;
3433

34+
import static java.nio.charset.StandardCharsets.UTF_8;
3535
import static org.assertj.core.api.Assertions.assertThat;
3636
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
3737
import static org.assertj.core.api.Assertions.catchThrowable;
@@ -72,7 +72,7 @@ void handleError() throws Exception {
7272
given(response.getStatusCode()).willReturn(HttpStatus.NOT_FOUND);
7373
given(response.getStatusText()).willReturn("Not Found");
7474
given(response.getHeaders()).willReturn(headers);
75-
given(response.getBody()).willReturn(new ByteArrayInputStream("Hello World".getBytes(StandardCharsets.UTF_8)));
75+
given(response.getBody()).willReturn(new ByteArrayInputStream("Hello World".getBytes(UTF_8)));
7676

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

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

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

107109
private void setupClientHttpResponse(HttpStatus status, @Nullable String textBody) throws Exception {
@@ -110,7 +112,7 @@ private void setupClientHttpResponse(HttpStatus status, @Nullable String textBod
110112
given(response.getStatusText()).willReturn(status.getReasonPhrase());
111113
if (textBody != null) {
112114
headers.setContentType(MediaType.TEXT_PLAIN);
113-
given(response.getBody()).willReturn(new ByteArrayInputStream(textBody.getBytes(StandardCharsets.UTF_8)));
115+
given(response.getBody()).willReturn(new ByteArrayInputStream(textBody.getBytes(UTF_8)));
114116
}
115117
given(response.getHeaders()).willReturn(headers);
116118
}
@@ -187,7 +189,7 @@ void handleErrorForCustomClientError() throws Exception {
187189
headers.setContentType(MediaType.TEXT_PLAIN);
188190

189191
String responseBody = "Hello World";
190-
TestByteArrayInputStream body = new TestByteArrayInputStream(responseBody.getBytes(StandardCharsets.UTF_8));
192+
TestByteArrayInputStream body = new TestByteArrayInputStream(responseBody.getBytes(UTF_8));
191193

192194
given(response.getStatusCode()).willReturn(statusCode);
193195
given(response.getStatusText()).willReturn(statusText);
@@ -227,7 +229,7 @@ void handleErrorForCustomServerError() throws Exception {
227229
headers.setContentType(MediaType.TEXT_PLAIN);
228230

229231
String responseBody = "Hello World";
230-
TestByteArrayInputStream body = new TestByteArrayInputStream(responseBody.getBytes(StandardCharsets.UTF_8));
232+
TestByteArrayInputStream body = new TestByteArrayInputStream(responseBody.getBytes(UTF_8));
231233

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

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

260262
assertThat(handler.hasError(response)).isFalse();
261263
assertThat(body.isClosed()).isFalse();
262-
assertThat(StreamUtils.copyToString(response.getBody(), StandardCharsets.UTF_8)).isEqualTo("Hello World");
264+
assertThat(StreamUtils.copyToString(response.getBody(), UTF_8)).isEqualTo("Hello World");
263265
}
264266

265267

0 commit comments

Comments
 (0)