DefaultResponseErrorHandler detects non-standard error code as well

Issue: SPR-17439

Author: jhoeller

spring-web/src/main/java/org/springframework/http/HttpStatus.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -419,58 +419,62 @@ public String getReasonPhrase() {
 		return this.reasonPhrase;
 	}
 
+	/**
+	 * Return the HTTP status series of this status code.
+	 * @see HttpStatus.Series
+	 */
+	public Series series() {
+		return Series.valueOf(this);
+	}
+
 	/**
 	 * Whether this status code is in the HTTP series
 	 * {@link org.springframework.http.HttpStatus.Series#INFORMATIONAL}.
 	 * This is a shortcut for checking the value of {@link #series()}.
+	 * @see #series()
 	 */
 	public boolean is1xxInformational() {
-		return Series.INFORMATIONAL.equals(series());
+		return (series() == Series.INFORMATIONAL);
 	}
 
 	/**
 	 * Whether this status code is in the HTTP series
 	 * {@link org.springframework.http.HttpStatus.Series#SUCCESSFUL}.
 	 * This is a shortcut for checking the value of {@link #series()}.
+	 * @see #series()
 	 */
 	public boolean is2xxSuccessful() {
-		return Series.SUCCESSFUL.equals(series());
+		return (series() == Series.SUCCESSFUL);
 	}
 
 	/**
 	 * Whether this status code is in the HTTP series
 	 * {@link org.springframework.http.HttpStatus.Series#REDIRECTION}.
 	 * This is a shortcut for checking the value of {@link #series()}.
+	 * @see #series()
 	 */
 	public boolean is3xxRedirection() {
-		return Series.REDIRECTION.equals(series());
+		return (series() == Series.REDIRECTION);
 	}
 
-
 	/**
 	 * Whether this status code is in the HTTP series
 	 * {@link org.springframework.http.HttpStatus.Series#CLIENT_ERROR}.
 	 * This is a shortcut for checking the value of {@link #series()}.
+	 * @see #series()
 	 */
 	public boolean is4xxClientError() {
-		return Series.CLIENT_ERROR.equals(series());
+		return (series() == Series.CLIENT_ERROR);
 	}
 
 	/**
 	 * Whether this status code is in the HTTP series
 	 * {@link org.springframework.http.HttpStatus.Series#SERVER_ERROR}.
 	 * This is a shortcut for checking the value of {@link #series()}.
+	 * @see #series()
 	 */
 	public boolean is5xxServerError() {
-		return Series.SERVER_ERROR.equals(series());
-	}
-
-	/**
-	 * Returns the HTTP status series of this status code.
-	 * @see HttpStatus.Series
-	 */
-	public Series series() {
-		return Series.valueOf(this);
+		return (series() == Series.SERVER_ERROR);
 	}
 
 	/**
@@ -523,18 +527,30 @@ public int value() {
 			return this.value;
 		}
 
-		public static Series valueOf(int status) {
-			int seriesCode = status / 100;
+		/**
+		 * Return the enum constant of this type with the corresponding series.
+		 * @param status a standard HTTP status enum value
+		 * @return the enum constant of this type with the corresponding series
+		 * @throws IllegalArgumentException if this enum has no corresponding constant
+		 */
+		public static Series valueOf(HttpStatus status) {
+			return valueOf(status.value);
+		}
+
+		/**
+		 * Return the enum constant of this type with the corresponding series.
+		 * @param statusCode the HTTP status code (potentially non-standard)
+		 * @return the enum constant of this type with the corresponding series
+		 * @throws IllegalArgumentException if this enum has no corresponding constant
+		 */
+		public static Series valueOf(int statusCode) {
+			int seriesCode = statusCode / 100;
 			for (Series series : values()) {
 				if (series.value == seriesCode) {
 					return series;
 				}
 			}
-			throw new IllegalArgumentException("No matching constant for [" + status + "]");
-		}
-
-		public static Series valueOf(HttpStatus status) {
-			return valueOf(status.value);
+			throw new IllegalArgumentException("No matching constant for [" + statusCode + "]");
 		}
 	}
 

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

@@ -43,7 +43,11 @@
 public class DefaultResponseErrorHandler implements ResponseErrorHandler {
 
 	/**
-	 * Delegates to {@link #hasError(HttpStatus)} with the response status code.
+	 * Delegates to {@link #hasError(HttpStatus)} (for a standard status enum value) or
+	 * {@link #hasError(int)} (for an unknown status code) with the response status code.
+	 * @see ClientHttpResponse#getRawStatusCode()
+	 * @see #hasError(HttpStatus)
+	 * @see #hasError(int)
 	 */
 	@Override
 	public boolean hasError(ClientHttpResponse response) throws IOException {
@@ -53,7 +57,7 @@ public boolean hasError(ClientHttpResponse response) throws IOException {
 				return hasError(statusCode);
 			}
 		}
-		return false;
+		return hasError(rawStatusCode);
 	}
 
 	/**
@@ -62,13 +66,31 @@ public boolean hasError(ClientHttpResponse response) throws IOException {
 	 * {@link HttpStatus.Series#CLIENT_ERROR CLIENT_ERROR} or
 	 * {@link HttpStatus.Series#SERVER_ERROR SERVER_ERROR}.
 	 * Can be overridden in subclasses.
-	 * @param statusCode the HTTP status code
-	 * @return {@code true} if the response has an error; {@code false} otherwise
-	 * @see #getHttpStatusCode(ClientHttpResponse)
+	 * @param statusCode the HTTP status code as enum value
+	 * @return {@code true} if the response indicates an error; {@code false} otherwise
+	 * @see HttpStatus#is4xxClientError()
+	 * @see HttpStatus#is5xxServerError()
 	 */
 	protected boolean hasError(HttpStatus statusCode) {
-		return (statusCode.series() == HttpStatus.Series.CLIENT_ERROR ||
-				statusCode.series() == HttpStatus.Series.SERVER_ERROR);
+		return (statusCode.is4xxClientError() || statusCode.is5xxServerError());
+	}
+
+	/**
+	 * Template method called from {@link #hasError(ClientHttpResponse)}.
+	 * <p>The default implementation checks if the given status code is
+	 * {@code HttpStatus.Series#CLIENT_ERROR CLIENT_ERROR} or
+	 * {@code HttpStatus.Series#SERVER_ERROR SERVER_ERROR}.
+	 * Can be overridden in subclasses.
+	 * @param unknownStatusCode the HTTP status code as raw value
+	 * @return {@code true} if the response indicates an error; {@code false} otherwise
+	 * @since 4.3.21
+	 * @see HttpStatus.Series#CLIENT_ERROR
+	 * @see HttpStatus.Series#SERVER_ERROR
+	 */
+	protected boolean hasError(int unknownStatusCode) {
+		int seriesCode = unknownStatusCode / 100;
+		return (seriesCode == HttpStatus.Series.CLIENT_ERROR.value() ||
+				seriesCode == HttpStatus.Series.SERVER_ERROR.value());
 	}
 
 	/**
@@ -93,7 +115,6 @@ public void handleError(ClientHttpResponse response) throws IOException {
 		}
 	}
 
-
 	/**
 	 * Determine the HTTP status of the given response.
 	 * <p>Note: Only called from {@link #handleError}, not from {@link #hasError}.

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2018 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -34,7 +34,7 @@ public interface ResponseErrorHandler {
 	 * <p>Implementations will typically inspect the
 	 * {@link ClientHttpResponse#getStatusCode() HttpStatus} of the response.
 	 * @param response the response to inspect
-	 * @return {@code true} if the response has an error; {@code false} otherwise
+	 * @return {@code true} if the response indicates an error; {@code false} otherwise
 	 * @throws IOException in case of I/O errors
 	 */
 	boolean hasError(ClientHttpResponse response) throws IOException;

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

@@ -103,8 +103,20 @@ public void handleErrorNullResponse() throws Exception {
 		handler.handleError(response);
 	}
 
+	@Test  // SPR-16108
+	public void hasErrorForUnknownStatusCode() throws Exception {
+		HttpHeaders headers = new HttpHeaders();
+		headers.setContentType(MediaType.TEXT_PLAIN);
+
+		given(response.getRawStatusCode()).willReturn(999);
+		given(response.getStatusText()).willReturn("Custom status code");
+		given(response.getHeaders()).willReturn(headers);
+
+		assertFalse(handler.hasError(response));
+	}
+
 	@Test(expected = UnknownHttpStatusCodeException.class)  // SPR-9406
-	public void unknownStatusCode() throws Exception {
+	public void handleErrorUnknownStatusCode() throws Exception {
 		HttpHeaders headers = new HttpHeaders();
 		headers.setContentType(MediaType.TEXT_PLAIN);
 
@@ -115,23 +127,59 @@ public void unknownStatusCode() throws Exception {
 		handler.handleError(response);
 	}
 
-	@Test  // SPR-16108
-	public void hasErrorForUnknownStatusCode() throws Exception {
+	@Test  // SPR-17461
+	public void hasErrorForCustomClientError() throws Exception {
 		HttpHeaders headers = new HttpHeaders();
 		headers.setContentType(MediaType.TEXT_PLAIN);
 
-		given(response.getRawStatusCode()).willReturn(999);
+		given(response.getRawStatusCode()).willReturn(499);
 		given(response.getStatusText()).willReturn("Custom status code");
 		given(response.getHeaders()).willReturn(headers);
 
-		assertFalse(handler.hasError(response));
+		assertTrue(handler.hasError(response));
+	}
+
+	@Test(expected = UnknownHttpStatusCodeException.class)
+	public void handleErrorForCustomClientError() throws Exception {
+		HttpHeaders headers = new HttpHeaders();
+		headers.setContentType(MediaType.TEXT_PLAIN);
+
+		given(response.getStatusCode()).willThrow(new IllegalArgumentException("No matching constant for 499"));
+		given(response.getStatusText()).willReturn("Custom status code");
+		given(response.getHeaders()).willReturn(headers);
+
+		handler.handleError(response);
+	}
+
+	@Test  // SPR-17461
+	public void hasErrorForCustomServerError() throws Exception {
+		HttpHeaders headers = new HttpHeaders();
+		headers.setContentType(MediaType.TEXT_PLAIN);
+
+		given(response.getRawStatusCode()).willReturn(599);
+		given(response.getStatusText()).willReturn("Custom status code");
+		given(response.getHeaders()).willReturn(headers);
+
+		assertTrue(handler.hasError(response));
+	}
+
+	@Test(expected = UnknownHttpStatusCodeException.class)
+	public void handleErrorForCustomServerError() throws Exception {
+		HttpHeaders headers = new HttpHeaders();
+		headers.setContentType(MediaType.TEXT_PLAIN);
+
+		given(response.getStatusCode()).willThrow(new IllegalArgumentException("No matching constant for 599"));
+		given(response.getStatusText()).willReturn("Custom status code");
+		given(response.getHeaders()).willReturn(headers);
+
+		handler.handleError(response);
 	}
 
 	@Test  // SPR-16604
 	public void bodyAvailableAfterHasErrorForUnknownStatusCode() throws Exception {
 		HttpHeaders headers = new HttpHeaders();
 		headers.setContentType(MediaType.TEXT_PLAIN);
-		TestByteArrayInputStream body = new TestByteArrayInputStream("Hello World".getBytes("UTF-8"));
+		TestByteArrayInputStream body = new TestByteArrayInputStream("Hello World".getBytes(UTF8));
 
 		given(response.getRawStatusCode()).willReturn(999);
 		given(response.getStatusText()).willReturn("Custom status code");