Skip to content

Commit 4a51acb

Browse files
committed
DefaultResponseErrorHandler detects non-standard error code as well
Issue: SPR-17439
1 parent 3d7e373 commit 4a51acb

File tree

4 files changed

+135
-45
lines changed

4 files changed

+135
-45
lines changed

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

Lines changed: 42 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2018 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.
@@ -435,68 +435,75 @@ public String getReasonPhrase() {
435435
return this.reasonPhrase;
436436
}
437437

438+
/**
439+
* Return the HTTP status series of this status code.
440+
* @see HttpStatus.Series
441+
*/
442+
public Series series() {
443+
return Series.valueOf(this);
444+
}
445+
438446
/**
439447
* Whether this status code is in the HTTP series
440448
* {@link org.springframework.http.HttpStatus.Series#INFORMATIONAL}.
441449
* This is a shortcut for checking the value of {@link #series()}.
450+
* @see #series()
442451
*/
443452
public boolean is1xxInformational() {
444-
return Series.INFORMATIONAL.equals(series());
453+
return (series() == Series.INFORMATIONAL);
445454
}
446455

447456
/**
448457
* Whether this status code is in the HTTP series
449458
* {@link org.springframework.http.HttpStatus.Series#SUCCESSFUL}.
450459
* This is a shortcut for checking the value of {@link #series()}.
460+
* @see #series()
451461
*/
452462
public boolean is2xxSuccessful() {
453-
return Series.SUCCESSFUL.equals(series());
463+
return (series() == Series.SUCCESSFUL);
454464
}
455465

456466
/**
457467
* Whether this status code is in the HTTP series
458468
* {@link org.springframework.http.HttpStatus.Series#REDIRECTION}.
459469
* This is a shortcut for checking the value of {@link #series()}.
470+
* @see #series()
460471
*/
461472
public boolean is3xxRedirection() {
462-
return Series.REDIRECTION.equals(series());
473+
return (series() == Series.REDIRECTION);
463474
}
464475

465-
466476
/**
467477
* Whether this status code is in the HTTP series
468478
* {@link org.springframework.http.HttpStatus.Series#CLIENT_ERROR}.
469479
* This is a shortcut for checking the value of {@link #series()}.
480+
* @see #series()
470481
*/
471482
public boolean is4xxClientError() {
472-
return Series.CLIENT_ERROR.equals(series());
483+
return (series() == Series.CLIENT_ERROR);
473484
}
474485

475486
/**
476487
* Whether this status code is in the HTTP series
477488
* {@link org.springframework.http.HttpStatus.Series#SERVER_ERROR}.
478489
* This is a shortcut for checking the value of {@link #series()}.
490+
* @see #series()
479491
*/
480492
public boolean is5xxServerError() {
481-
return Series.SERVER_ERROR.equals(series());
493+
return (series() == Series.SERVER_ERROR);
482494
}
483495

484496
/**
485497
* Whether this status code is in the HTTP series
486498
* {@link org.springframework.http.HttpStatus.Series#CLIENT_ERROR} or
487499
* {@link org.springframework.http.HttpStatus.Series#SERVER_ERROR}.
488500
* This is a shortcut for checking the value of {@link #series()}.
501+
* @since 5.0
502+
* @see #is4xxClientError()
503+
* @see #is5xxServerError()
489504
*/
490505
public boolean isError() {
491-
return is4xxClientError() || is5xxServerError();
492-
}
493-
494-
/**
495-
* Returns the HTTP status series of this status code.
496-
* @see HttpStatus.Series
497-
*/
498-
public Series series() {
499-
return Series.valueOf(this);
506+
return (is4xxClientError() || is5xxServerError());
500507
}
501508

502509
/**
@@ -522,7 +529,6 @@ public static HttpStatus valueOf(int statusCode) {
522529
return status;
523530
}
524531

525-
526532
/**
527533
* Resolve the given status code to an {@code HttpStatus}, if possible.
528534
* @param statusCode the HTTP status code (potentially non-standard)
@@ -565,18 +571,30 @@ public int value() {
565571
return this.value;
566572
}
567573

568-
public static Series valueOf(int status) {
569-
int seriesCode = status / 100;
574+
/**
575+
* Return the enum constant of this type with the corresponding series.
576+
* @param status a standard HTTP status enum value
577+
* @return the enum constant of this type with the corresponding series
578+
* @throws IllegalArgumentException if this enum has no corresponding constant
579+
*/
580+
public static Series valueOf(HttpStatus status) {
581+
return valueOf(status.value);
582+
}
583+
584+
/**
585+
* Return the enum constant of this type with the corresponding series.
586+
* @param statusCode the HTTP status code (potentially non-standard)
587+
* @return the enum constant of this type with the corresponding series
588+
* @throws IllegalArgumentException if this enum has no corresponding constant
589+
*/
590+
public static Series valueOf(int statusCode) {
591+
int seriesCode = statusCode / 100;
570592
for (Series series : values()) {
571593
if (series.value == seriesCode) {
572594
return series;
573595
}
574596
}
575-
throw new IllegalArgumentException("No matching constant for [" + status + "]");
576-
}
577-
578-
public static Series valueOf(HttpStatus status) {
579-
return valueOf(status.value);
597+
throw new IllegalArgumentException("No matching constant for [" + statusCode + "]");
580598
}
581599
}
582600

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

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,29 @@
4444
public class DefaultResponseErrorHandler implements ResponseErrorHandler {
4545

4646
/**
47-
* Delegates to {@link #hasError(HttpStatus)} with the response status code.
47+
* Delegates to {@link #hasError(HttpStatus)} (for a standard status enum value) or
48+
* {@link #hasError(int)} (for an unknown status code) with the response status code.
49+
* @see ClientHttpResponse#getRawStatusCode()
50+
* @see #hasError(HttpStatus)
51+
* @see #hasError(int)
4852
*/
4953
@Override
5054
public boolean hasError(ClientHttpResponse response) throws IOException {
51-
HttpStatus statusCode = HttpStatus.resolve(response.getRawStatusCode());
52-
return (statusCode != null && hasError(statusCode));
55+
int rawStatusCode = response.getRawStatusCode();
56+
HttpStatus statusCode = HttpStatus.resolve(rawStatusCode);
57+
return (statusCode != null ? hasError(statusCode) : hasError(rawStatusCode));
58+
}
59+
60+
/**
61+
* Template method called from {@link #hasError(ClientHttpResponse)}.
62+
* <p>The default implementation checks {@link HttpStatus#isError()}.
63+
* Can be overridden in subclasses.
64+
* @param statusCode the HTTP status code as enum value
65+
* @return {@code true} if the response indicates an error; {@code false} otherwise
66+
* @see HttpStatus#isError()
67+
*/
68+
protected boolean hasError(HttpStatus statusCode) {
69+
return statusCode.isError();
5370
}
5471

5572
/**
@@ -58,16 +75,23 @@ public boolean hasError(ClientHttpResponse response) throws IOException {
5875
* {@code HttpStatus.Series#CLIENT_ERROR CLIENT_ERROR} or
5976
* {@code HttpStatus.Series#SERVER_ERROR SERVER_ERROR}.
6077
* Can be overridden in subclasses.
61-
* @param statusCode the HTTP status code
62-
* @return {@code true} if the response has an error; {@code false} otherwise
78+
* @param unknownStatusCode the HTTP status code as raw value
79+
* @return {@code true} if the response indicates an error; {@code false} otherwise
80+
* @since 4.3.21
81+
* @see HttpStatus.Series#CLIENT_ERROR
82+
* @see HttpStatus.Series#SERVER_ERROR
6383
*/
64-
protected boolean hasError(HttpStatus statusCode) {
65-
return (statusCode.series() == HttpStatus.Series.CLIENT_ERROR ||
66-
statusCode.series() == HttpStatus.Series.SERVER_ERROR);
84+
protected boolean hasError(int unknownStatusCode) {
85+
int seriesCode = unknownStatusCode / 100;
86+
return (seriesCode == HttpStatus.Series.CLIENT_ERROR.value() ||
87+
seriesCode == HttpStatus.Series.SERVER_ERROR.value());
6788
}
6889

6990
/**
70-
* Delegates to {@link #handleError(ClientHttpResponse, HttpStatus)} with the response status code.
91+
* Delegates to {@link #handleError(ClientHttpResponse, HttpStatus)} with the
92+
* response status code.
93+
* @throws UnknownHttpStatusCodeException in case of an unresolvable status code
94+
* @see #handleError(ClientHttpResponse, HttpStatus)
7195
*/
7296
@Override
7397
public void handleError(ClientHttpResponse response) throws IOException {
@@ -81,10 +105,10 @@ public void handleError(ClientHttpResponse response) throws IOException {
81105

82106
/**
83107
* Handle the error in the given response with the given resolved status code.
84-
* <p>This default implementation throws a {@link HttpClientErrorException} if the response status code
85-
* is {@link org.springframework.http.HttpStatus.Series#CLIENT_ERROR}, a {@link HttpServerErrorException}
86-
* if it is {@link org.springframework.http.HttpStatus.Series#SERVER_ERROR},
87-
* and a {@link RestClientException} in other cases.
108+
* <p>The default implementation throws an {@link HttpClientErrorException}
109+
* if the status code is {@link HttpStatus.Series#CLIENT_ERROR}, an
110+
* {@link HttpServerErrorException} if it is {@link HttpStatus.Series#SERVER_ERROR},
111+
* and an {@link UnknownHttpStatusCodeException} in other cases.
88112
* @since 5.0
89113
*/
90114
protected void handleError(ClientHttpResponse response, HttpStatus statusCode) throws IOException {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2018 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.
@@ -36,7 +36,7 @@ public interface ResponseErrorHandler {
3636
* <p>Implementations will typically inspect the
3737
* {@link ClientHttpResponse#getStatusCode() HttpStatus} of the response.
3838
* @param response the response to inspect
39-
* @return {@code true} if the response has an error; {@code false} otherwise
39+
* @return {@code true} if the response indicates an error; {@code false} otherwise
4040
* @throws IOException in case of I/O errors
4141
*/
4242
boolean hasError(ClientHttpResponse response) throws IOException;

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)