DefaultResponseErrorHandler updates

Deprecate handleError(response), and ensure it continues to be invoked
if overridden.

See gh-34231

Author: rstoyanchev

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

@@ -49,8 +49,8 @@
  * {@link #hasError(HttpStatusCode)}. Unknown status codes will be ignored by
  * {@link #hasError(ClientHttpResponse)}.
  *
- * <p>See {@link #handleError(ClientHttpResponse)} for more details on specific
- * exception types.
+ * <p>See {@link #handleError(URI, HttpMethod, ClientHttpResponse)}  for more
+ * details on specific exception types.
  *
  * @author Arjen Poutsma
  * @author Rossen Stoyanchev
@@ -116,27 +116,6 @@ protected boolean hasError(int statusCode) {
 		return (series == HttpStatus.Series.CLIENT_ERROR || series == HttpStatus.Series.SERVER_ERROR);
 	}
 
-	/**
-	 * 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(ClientHttpResponse, HttpStatusCode, URI, HttpMethod)
-	 */
-	@Override
-	public void handleError(ClientHttpResponse response) throws IOException {
-		handleError(response, response.getStatusCode(), null, null);
-	}
-
 	/**
 	 * Handle the error in the given response with the given resolved status code
 	 * and extra information providing access to the request URL and HTTP method.
@@ -157,9 +136,20 @@ public void handleError(ClientHttpResponse response) throws IOException {
 	 */
 	@Override
 	public void handleError(URI url, HttpMethod method, ClientHttpResponse response) throws IOException {
+		handleError(response);
 		handleError(response, response.getStatusCode(), url, method);
 	}
 
+	/**
+	 * {@inheritDoc}
+	 * <p>As of 6.2.1 this method is a no-op unless overridden.
+	 */
+	@SuppressWarnings("removal")
+	@Override
+	public void handleError(ClientHttpResponse response) throws IOException {
+		// no-op, but here for backwards compatibility
+	}
+
 	/**
 	 * Handle the error based on the resolved status code.
 	 * <p>The default implementation delegates to

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

@@ -42,11 +42,12 @@
  * 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}.
+ * {@link #handleError(ClientHttpResponse, HttpStatusCode, URI, HttpMethod)}
+ * 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
@@ -98,9 +99,10 @@ public void setMessageConverters(List<HttpMessageConverter<?>> messageConverters
 	 * If this mapping 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}.
+	 * {@code true} and {@link #handleError(ClientHttpResponse, HttpStatusCode, URI, HttpMethod)}
+	 * will attempt to use the {@linkplain #setMessageConverters(List) configured
+	 * message converters} to convert the response into the mapped subclass of
+	 * {@link RestClientException}.
 	 */
 	public void setStatusMapping(Map<HttpStatusCode, Class<? extends RestClientException>> statusMapping) {
 		if (!CollectionUtils.isEmpty(statusMapping)) {
@@ -113,9 +115,10 @@ public void setStatusMapping(Map<HttpStatusCode, Class<? extends RestClientExcep
 	 * If this mapping 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}.
+	 * {@code true} and {@link #handleError(ClientHttpResponse, HttpStatusCode, URI, HttpMethod)}
+	 * will attempt to use the {@linkplain #setMessageConverters(List) configured
+	 * message converters} to convert the response into the mapped subclass of
+	 * {@link RestClientException}.
 	 */
 	public void setSeriesMapping(Map<HttpStatus.Series, Class<? extends RestClientException>> seriesMapping) {
 		if (!CollectionUtils.isEmpty(seriesMapping)) {

spring-web/src/main/java/org/springframework/web/client/NoOpResponseErrorHandler.java

@@ -43,9 +43,4 @@ public boolean hasError(ClientHttpResponse response) throws IOException {
 		return false;
 	}
 
-	@Override
-	public void handleError(ClientHttpResponse response) throws IOException {
-		// never actually called
-	}
-
 }

spring-web/src/main/java/org/springframework/web/client/ResponseErrorHandler.java

@@ -45,14 +45,6 @@ public interface ResponseErrorHandler {
 	 * Handle the error in the given response.
 	 * <p>This method is only called when {@link #hasError(ClientHttpResponse)}
 	 * has returned {@code true}.
-	 * @param response the response with the error
-	 * @throws IOException in case of I/O errors
-	 */
-	void handleError(ClientHttpResponse response) throws IOException;
-
-	/**
-	 * Alternative to {@link #handleError(ClientHttpResponse)} with extra
-	 * information providing access to the request URL and HTTP method.
 	 * @param url the request URL
 	 * @param method the HTTP method
 	 * @param response the response with the error
@@ -63,4 +55,17 @@ default void handleError(URI url, HttpMethod method, ClientHttpResponse response
 		handleError(response);
 	}
 
+	/**
+	 * Handle the error in the given response.
+	 * <p>This method is only called when {@link #hasError(ClientHttpResponse)}
+	 * has returned {@code true}.
+	 * @param response the response with the error
+	 * @throws IOException in case of I/O errors
+	 * @deprecated in favor of {@link #handleError(URI, HttpMethod, ClientHttpResponse)}
+	 */
+	@Deprecated(since = "6.2.1", forRemoval = true)
+	default void handleError(ClientHttpResponse response) throws IOException {
+		// no-op unless overridden
+	}
+
 }

spring-web/src/test/java/org/springframework/web/client/DefaultResponseErrorHandlerHttpStatusTests.java

@@ -16,6 +16,7 @@
 
 package org.springframework.web.client;
 
+import java.net.URI;
 import java.util.stream.Stream;
 
 import org.junit.jupiter.api.DisplayName;
@@ -24,6 +25,7 @@
 import org.junit.jupiter.params.provider.MethodSource;
 
 import org.springframework.http.HttpHeaders;
+import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.MediaType;
 import org.springframework.http.client.ClientHttpResponse;
@@ -80,7 +82,8 @@ void handleErrorException(HttpStatus httpStatus, Class<? extends Throwable> expe
 		given(this.response.getStatusCode()).willReturn(httpStatus);
 		given(this.response.getHeaders()).willReturn(headers);
 
-		assertThatExceptionOfType(expectedExceptionClass).isThrownBy(() -> this.handler.handleError(this.response));
+		assertThatExceptionOfType(expectedExceptionClass)
+				.isThrownBy(() -> this.handler.handleError(URI.create("/"), HttpMethod.GET, this.response));
 	}
 
 	static Stream<Arguments> errorCodes() {

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

@@ -75,8 +75,8 @@ void handleError() throws Exception {
 		given(response.getBody()).willReturn(new ByteArrayInputStream("Hello World".getBytes(UTF_8)));
 
 		assertThatExceptionOfType(HttpClientErrorException.class)
-				.isThrownBy(() -> handler.handleError(response))
-				.withMessage("404 Not Found: \"Hello World\"")
+				.isThrownBy(() -> handler.handleError(URI.create("/"), HttpMethod.GET, response))
+				.withMessage("404 Not Found on GET request for \"/\": \"Hello World\"")
 				.satisfies(ex -> assertThat(ex.getResponseHeaders()).isEqualTo(headers));
 	}
 
@@ -127,7 +127,8 @@ void handleErrorIOException() throws Exception {
 		given(response.getHeaders()).willReturn(headers);
 		given(response.getBody()).willThrow(new IOException());
 
-		assertThatExceptionOfType(HttpClientErrorException.class).isThrownBy(() -> handler.handleError(response));
+		assertThatExceptionOfType(HttpClientErrorException.class)
+				.isThrownBy(() -> handler.handleError(URI.create("/"), HttpMethod.GET, response));
 	}
 
 	@Test
@@ -140,7 +141,7 @@ void handleErrorNullResponse() throws Exception {
 		given(response.getHeaders()).willReturn(headers);
 
 		assertThatExceptionOfType(HttpClientErrorException.class).isThrownBy(() ->
-				handler.handleError(response));
+				handler.handleError(URI.create("/"), HttpMethod.GET, response));
 	}
 
 	@Test  // SPR-16108
@@ -165,7 +166,7 @@ public void handleErrorUnknownStatusCode() throws Exception {
 		given(response.getHeaders()).willReturn(headers);
 
 		assertThatExceptionOfType(UnknownHttpStatusCodeException.class).isThrownBy(() ->
-				handler.handleError(response));
+				handler.handleError(URI.create("/"), HttpMethod.GET, response));
 	}
 
 	@Test  // SPR-17461
@@ -196,7 +197,7 @@ void handleErrorForCustomClientError() throws Exception {
 		given(response.getHeaders()).willReturn(headers);
 		given(response.getBody()).willReturn(body);
 
-		Throwable throwable = catchThrowable(() -> handler.handleError(response));
+		Throwable throwable = catchThrowable(() -> handler.handleError(URI.create("/"), HttpMethod.GET, response));
 
 		// validate exception
 		assertThat(throwable).isInstanceOf(HttpClientErrorException.class);
@@ -236,7 +237,7 @@ void handleErrorForCustomServerError() throws Exception {
 		given(response.getHeaders()).willReturn(headers);
 		given(response.getBody()).willReturn(body);
 
-		Throwable throwable = catchThrowable(() -> handler.handleError(response));
+		Throwable throwable = catchThrowable(() -> handler.handleError(URI.create("/"), HttpMethod.GET, response));
 
 		// validate exception
 		assertThat(throwable).isInstanceOf(HttpServerErrorException.class);

spring-web/src/test/java/org/springframework/web/client/ExtractingResponseErrorHandlerTests.java

@@ -17,6 +17,7 @@
 package org.springframework.web.client;
 
 import java.io.ByteArrayInputStream;
+import java.net.URI;
 import java.nio.charset.StandardCharsets;
 import java.util.Collections;
 import java.util.List;
@@ -26,6 +27,7 @@
 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.MediaType;
 import org.springframework.http.client.ClientHttpResponse;
@@ -98,7 +100,7 @@ void handleErrorStatusMatch() throws Exception {
 		given(this.response.getBody()).willReturn(new ByteArrayInputStream(body));
 
 		assertThatExceptionOfType(MyRestClientException.class)
-				.isThrownBy(() -> this.errorHandler.handleError(this.response))
+				.isThrownBy(() -> this.errorHandler.handleError(URI.create("/"), HttpMethod.GET, this.response))
 				.satisfies(ex -> assertThat(ex.getFoo()).isEqualTo("bar"));
 	}
 
@@ -114,7 +116,7 @@ void handleErrorSeriesMatch() throws Exception {
 		given(this.response.getBody()).willReturn(new ByteArrayInputStream(body));
 
 		assertThatExceptionOfType(MyRestClientException.class)
-				.isThrownBy(() -> this.errorHandler.handleError(this.response))
+				.isThrownBy(() -> this.errorHandler.handleError(URI.create("/"), HttpMethod.GET, this.response))
 				.satisfies(ex -> assertThat(ex.getFoo()).isEqualTo("bar"));
 	}
 
@@ -130,7 +132,7 @@ void handleNoMatch() throws Exception {
 		given(this.response.getBody()).willReturn(new ByteArrayInputStream(body));
 
 		assertThatExceptionOfType(HttpClientErrorException.class)
-				.isThrownBy(() -> this.errorHandler.handleError(this.response))
+				.isThrownBy(() -> this.errorHandler.handleError(URI.create("/"), HttpMethod.GET, this.response))
 				.satisfies(ex -> {
 					assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND);
 					assertThat(ex.getResponseBodyAsByteArray()).isEqualTo(body);
@@ -150,7 +152,7 @@ void handleNoMatchOverride() throws Exception {
 		responseHeaders.setContentLength(body.length);
 		given(this.response.getBody()).willReturn(new ByteArrayInputStream(body));
 
-		this.errorHandler.handleError(this.response);
+		this.errorHandler.handleError(URI.create("/"), HttpMethod.GET, this.response);
 	}
 
 

spring-web/src/test/java/org/springframework/web/client/RestClientObservationTests.java

@@ -345,12 +345,12 @@ static class ObservationErrorHandler implements ResponseErrorHandler {
 		}
 
 		@Override
-		public boolean hasError(ClientHttpResponse response) throws IOException {
+		public boolean hasError(ClientHttpResponse response) {
 			return true;
 		}
 
 		@Override
-		public void handleError(ClientHttpResponse response) throws IOException {
+		public void handleError(URI uri, HttpMethod httpMethod, ClientHttpResponse response) {
 			assertThat(this.observationRegistry.getCurrentObservationScope()).isNotNull();
 		}
 	}

spring-web/src/test/java/org/springframework/web/client/RestTemplateObservationTests.java

@@ -242,12 +242,12 @@ static class ObservationErrorHandler implements ResponseErrorHandler {
 		}
 
 		@Override
-		public boolean hasError(ClientHttpResponse response) throws IOException {
+		public boolean hasError(ClientHttpResponse response) {
 			return true;
 		}
 
 		@Override
-		public void handleError(ClientHttpResponse response) throws IOException {
+		public void handleError(URI uri, HttpMethod httpMethod, ClientHttpResponse response) {
 			currentObservation = this.observationRegistry.getCurrentObservation();
 		}
 	}