Skip to content

Commit 738097d

Browse files
committed
DefaultResponseErrorHandler detects non-standard error code as well
Issue: SPR-17439
1 parent b90553d commit 738097d

File tree

4 files changed

+115
-26
lines changed

4 files changed

+115
-26
lines changed

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

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -522,7 +522,6 @@ public static HttpStatus valueOf(int statusCode) {
522522
return status;
523523
}
524524

525-
526525
/**
527526
* Resolve the given status code to an {@code HttpStatus}, if possible.
528527
* @param statusCode the HTTP status code (potentially non-standard)
@@ -565,18 +564,45 @@ public int value() {
565564
return this.value;
566565
}
567566

568-
public static Series valueOf(int status) {
569-
int seriesCode = status / 100;
567+
/**
568+
* Return the enum constant of this type with the corresponding series.
569+
* @param status a standard HTTP status enum value
570+
* @return the enum constant of this type with the corresponding series
571+
* @throws IllegalArgumentException if this enum has no corresponding constant
572+
*/
573+
public static Series valueOf(HttpStatus status) {
574+
return valueOf(status.value);
575+
}
576+
577+
/**
578+
* Return the enum constant of this type with the corresponding series.
579+
* @param statusCode the HTTP status code (potentially non-standard)
580+
* @return the enum constant of this type with the corresponding series
581+
* @throws IllegalArgumentException if this enum has no corresponding constant
582+
*/
583+
public static Series valueOf(int statusCode) {
584+
Series series = resolve(statusCode);
585+
if (series == null) {
586+
throw new IllegalArgumentException("No matching constant for [" + statusCode + "]");
587+
}
588+
return series;
589+
}
590+
591+
/**
592+
* Resolve the given status code to an {@code HttpStatus.Series}, if possible.
593+
* @param statusCode the HTTP status code (potentially non-standard)
594+
* @return the corresponding {@code Series}, or {@code null} if not found
595+
* @since 5.1.3
596+
*/
597+
@Nullable
598+
public static Series resolve(int statusCode) {
599+
int seriesCode = statusCode / 100;
570600
for (Series series : values()) {
571601
if (series.value == seriesCode) {
572602
return series;
573603
}
574604
}
575-
throw new IllegalArgumentException("No matching constant for [" + status + "]");
576-
}
577-
578-
public static Series valueOf(HttpStatus status) {
579-
return valueOf(status.value);
605+
return null;
580606
}
581607
}
582608

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

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,9 @@ public class DefaultResponseErrorHandler implements ResponseErrorHandler {
4848
*/
4949
@Override
5050
public boolean hasError(ClientHttpResponse response) throws IOException {
51-
HttpStatus statusCode = HttpStatus.resolve(response.getRawStatusCode());
52-
return (statusCode != null && hasError(statusCode));
51+
int rawStatusCode = response.getRawStatusCode();
52+
HttpStatus statusCode = HttpStatus.resolve(rawStatusCode);
53+
return (statusCode != null ? hasError(statusCode) : hasError(rawStatusCode));
5354
}
5455

5556
/**
@@ -58,14 +59,29 @@ public boolean hasError(ClientHttpResponse response) throws IOException {
5859
* {@code HttpStatus.Series#CLIENT_ERROR CLIENT_ERROR} or
5960
* {@code HttpStatus.Series#SERVER_ERROR SERVER_ERROR}.
6061
* Can be overridden in subclasses.
61-
* @param statusCode the HTTP status code
62+
* @param statusCode the HTTP status code as enum value
6263
* @return {@code true} if the response has an error; {@code false} otherwise
6364
*/
6465
protected boolean hasError(HttpStatus statusCode) {
6566
return (statusCode.series() == HttpStatus.Series.CLIENT_ERROR ||
6667
statusCode.series() == HttpStatus.Series.SERVER_ERROR);
6768
}
6869

70+
/**
71+
* Template method called from {@link #hasError(ClientHttpResponse)}.
72+
* <p>The default implementation checks if the given status code is
73+
* {@code HttpStatus.Series#CLIENT_ERROR CLIENT_ERROR} or
74+
* {@code HttpStatus.Series#SERVER_ERROR SERVER_ERROR}.
75+
* Can be overridden in subclasses.
76+
* @param unknownStatusCode the HTTP status code as raw value
77+
* @return {@code true} if the response has an error; {@code false} otherwise
78+
* @since 4.3.21
79+
*/
80+
protected boolean hasError(int unknownStatusCode) {
81+
HttpStatus.Series series = HttpStatus.Series.resolve(unknownStatusCode);
82+
return (series == HttpStatus.Series.CLIENT_ERROR || series == HttpStatus.Series.SERVER_ERROR);
83+
}
84+
6985
/**
7086
* Delegates to {@link #handleError(ClientHttpResponse, HttpStatus)} with the response status code.
7187
*/

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

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828

2929
import static org.junit.Assert.*;
3030
import static org.mockito.BDDMockito.*;
31-
import static org.mockito.Mockito.mock;
3231
import static org.springframework.http.HttpStatus.*;
3332

3433
/**
@@ -66,15 +65,21 @@ public static Object[][] errorCodes() {
6665
public HttpStatus httpStatus;
6766

6867
@Parameterized.Parameter(1)
69-
public Class expectedExceptionClass;
68+
public Class<? extends Throwable> expectedExceptionClass;
7069

7170
private final DefaultResponseErrorHandler handler = new DefaultResponseErrorHandler();
7271

7372
private final ClientHttpResponse response = mock(ClientHttpResponse.class);
7473

7574

7675
@Test
77-
public void handleErrorIOException() throws Exception {
76+
public void hasErrorTrue() throws Exception {
77+
given(this.response.getRawStatusCode()).willReturn(this.httpStatus.value());
78+
assertTrue(this.handler.hasError(this.response));
79+
}
80+
81+
@Test
82+
public void handleErrorException() throws Exception {
7883
HttpHeaders headers = new HttpHeaders();
7984
headers.setContentType(MediaType.TEXT_PLAIN);
8085

@@ -91,10 +96,4 @@ public void handleErrorIOException() throws Exception {
9196
}
9297
}
9398

94-
@Test
95-
public void hasErrorTrue() throws Exception {
96-
given(this.response.getRawStatusCode()).willReturn(HttpStatus.NOT_FOUND.value());
97-
assertTrue(handler.hasError(this.response));
98-
}
99-
10099
}

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

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public void handleError() throws Exception {
6565
given(response.getRawStatusCode()).willReturn(HttpStatus.NOT_FOUND.value());
6666
given(response.getStatusText()).willReturn("Not Found");
6767
given(response.getHeaders()).willReturn(headers);
68-
given(response.getBody()).willReturn(new ByteArrayInputStream("Hello World".getBytes("UTF-8")));
68+
given(response.getBody()).willReturn(new ByteArrayInputStream("Hello World".getBytes(StandardCharsets.UTF_8)));
6969

7070
try {
7171
handler.handleError(response);
@@ -101,8 +101,20 @@ public void handleErrorNullResponse() throws Exception {
101101
handler.handleError(response);
102102
}
103103

104+
@Test // SPR-16108
105+
public void hasErrorForUnknownStatusCode() throws Exception {
106+
HttpHeaders headers = new HttpHeaders();
107+
headers.setContentType(MediaType.TEXT_PLAIN);
108+
109+
given(response.getRawStatusCode()).willReturn(999);
110+
given(response.getStatusText()).willReturn("Custom status code");
111+
given(response.getHeaders()).willReturn(headers);
112+
113+
assertFalse(handler.hasError(response));
114+
}
115+
104116
@Test(expected = UnknownHttpStatusCodeException.class) // SPR-9406
105-
public void unknownStatusCode() throws Exception {
117+
public void handleErrorUnknownStatusCode() throws Exception {
106118
HttpHeaders headers = new HttpHeaders();
107119
headers.setContentType(MediaType.TEXT_PLAIN);
108120

@@ -113,16 +125,52 @@ public void unknownStatusCode() throws Exception {
113125
handler.handleError(response);
114126
}
115127

116-
@Test // SPR-16108
117-
public void hasErrorForUnknownStatusCode() throws Exception {
128+
@Test // SPR-17461
129+
public void hasErrorForCustomClientError() throws Exception {
118130
HttpHeaders headers = new HttpHeaders();
119131
headers.setContentType(MediaType.TEXT_PLAIN);
120132

121-
given(response.getRawStatusCode()).willReturn(999);
133+
given(response.getRawStatusCode()).willReturn(499);
122134
given(response.getStatusText()).willReturn("Custom status code");
123135
given(response.getHeaders()).willReturn(headers);
124136

125-
assertFalse(handler.hasError(response));
137+
assertTrue(handler.hasError(response));
138+
}
139+
140+
@Test(expected = UnknownHttpStatusCodeException.class)
141+
public void handleErrorForCustomClientError() throws Exception {
142+
HttpHeaders headers = new HttpHeaders();
143+
headers.setContentType(MediaType.TEXT_PLAIN);
144+
145+
given(response.getRawStatusCode()).willReturn(499);
146+
given(response.getStatusText()).willReturn("Custom status code");
147+
given(response.getHeaders()).willReturn(headers);
148+
149+
handler.handleError(response);
150+
}
151+
152+
@Test // SPR-17461
153+
public void hasErrorForCustomServerError() throws Exception {
154+
HttpHeaders headers = new HttpHeaders();
155+
headers.setContentType(MediaType.TEXT_PLAIN);
156+
157+
given(response.getRawStatusCode()).willReturn(599);
158+
given(response.getStatusText()).willReturn("Custom status code");
159+
given(response.getHeaders()).willReturn(headers);
160+
161+
assertTrue(handler.hasError(response));
162+
}
163+
164+
@Test(expected = UnknownHttpStatusCodeException.class)
165+
public void handleErrorForCustomServerError() throws Exception {
166+
HttpHeaders headers = new HttpHeaders();
167+
headers.setContentType(MediaType.TEXT_PLAIN);
168+
169+
given(response.getRawStatusCode()).willReturn(599);
170+
given(response.getStatusText()).willReturn("Custom status code");
171+
given(response.getHeaders()).willReturn(headers);
172+
173+
handler.handleError(response);
126174
}
127175

128176
@Test // SPR-16604

0 commit comments

Comments
 (0)