[SPR-9406] Unknown status codes (i.e. not in HttpStatus enum) prevent HttpClientErrorException and HttpServerErrorExceptions from being raised Created: 14/May/12  Updated: 15/Jan/19  Resolved: 02/Nov/12

Status: Closed
Project: Spring Framework
Component/s: Web, Web:Client
Affects Version/s: 3.1.1
Fix Version/s: 3.2 RC1
Security Level: Public

Type: Bug Priority: Minor
Reporter: harish Assignee: Rossen Stoyanchev
Resolution: Complete Votes: 0
Labels: None
Σ Remaining Estimate: Not Specified Remaining Estimate: Not Specified
Σ Time Spent: Not Specified Time Spent: Not Specified
Σ Original Estimate: Not Specified Original Estimate: Not Specified

Issue Links:
Relate
is related to SPR-6752 RestTemplate throws IllegalArgumentEx... Closed
is related to SPR-15978 RestTemplate doesn't consistently tol... Closed
Sub-Tasks:
Key
Summary
Type
Status
Assignee
SPR-9502 Backport "Unknown status codes (i.e. ... Backport Resolved Rossen Stoyanchev  
Days since last comment: 1 year, 1 week, 6 days ago
Last commented by a User: true
Last updater: Spring Issuemaster

 Description   

RestTemplates HTTP status codes to only the ones that have associated messages. All 1xx,2xx,3xx,4xx,5xx are legal and valid response codes. RestTempllate should treat them as legal and not throw IllegalArgumentException or ignore message body when any of them are passed. Both code and body should be preserved and passed to caller as such. That would help services and clients make use of flexibility provided by HTTP specifications in using existing status codes and adding their own if no suitable status exists for their needs. This is preventing us from using RestTemplate.



 Comments   
Comment by Rossen Stoyanchev [ 15/May/12 ]

The RestTemplate delegates to a ResponseErrorHandler to determine if a particular response is an error. The DefaultResponseErrorHandler raises a HttpClientErrorException or a HttpServerErrorException (not IllegalArgumentException). Both exceptions contain and provide access to the status code and the response body. You can override the handleError(..) method in DefaultResponseErrorHandler and configure the RestTemplate with it.

Comment by harish [ 15/May/12 ]

ResponseErrorHandler internally calls response.getStatusCode() below, that internally calls valueOf in HttpStatus class, HttpStatus class maintains enum of only few status codes and if response does not have one of them it throws IllegalArgumentException and all response data including status and responseBody is lost.

public HttpStatus getStatusCode()

{ return HttpStatus.valueOf(httpMethod.getStatusCode()); }

HttpStatus class
private static Series valueOf(HttpStatus status) {
int seriesCode = status.value() / 100;
for (Series series : values()) {
if (series.value == seriesCode)

{ return series; }

}
throw new IllegalArgumentException("No matching constant for [" + status + "]");
}

(Also, Additionally, HttpClientErrorException does not contain response body, just the status text ---- throw new HttpClientErrorException(statusCode, response.getStatusText()) )

Comment by harish [ 15/May/12 ]

Just to be clear, if status code is not one of that in httpstatus enum, excution does not even reach to throw HttpClientErrorException and fails while executing response.getStatusCode(). Overriding of DefaultResponseErrorHandler does not help, as we can't get the statuscode from ClientHttpResponse and call fails with illegal argumentException.

Comment by Rossen Stoyanchev [ 15/May/12 ]

HttpClientErrorException does not contain response body, just the status text

Are we looking at the same source code? See DefaultResponseErrorHandler.java, line 69

You have a point about the status code. The DefaultResponseErrorHandler on line 80 throws a RestClientException for unknown status codes but it does look like that line wouldn't be reached. So I'll have a look into that.

Overriding of DefaultResponseErrorHandler does not help, as we can't get the statuscode from ClientHttpResponse

Have you tried response.getRawCode()?

Comment by Rossen Stoyanchev [ 15/May/12 ]

Modified title (was: "Allow user-defined HTTP status codes in RestTemplate")

Comment by harish [ 15/May/12 ]

Thanks Rossen. I had slightly older version of code, you are right, and it does look like now body will be preserved in exception.

Also, tue that getRawCode will now provide the workaround, but we have many clients and asking them all to modify the client code (to override DefaultResponseErrorHandler) may not be ideal. In build support for handling all status codes will be great help in easily using the RestTemplate.

Thanks for looking into it.

-Harish

Comment by Rossen Stoyanchev [ 13/Jun/12 ]

The DefaultResponseErrorHandler now raises a RestClientException rather than an IllegalArgumentException in case of an unknown status code. This doesn't provide access to the body and the headers but is consistent in terms of the exceptions raised. Note that the same exception is also raised if we don't recognize the status code as either client or server specific, which should be even more rare.

Comment by harish [ 14/Jun/12 ]

Thanks so much for looking into it and working on it. I am afraid though that this does not resolve the original issue of lost response body (header does get preserved now, which is good) in case of status code without default message. Opening it again. Is it impossible to preserve the response body in this scenario?

Comment by harish [ 14/Jun/12 ]

Would adding and setting headers and body in the RestClientException similar to HttpClientErrorException and HttpServerErrorException get us there? Is there any harm setting them there?

Comment by Rossen Stoyanchev [ 14/Jun/12 ]

RestClientException is more general and is thrown in places where headers and body are not available. The HttpStatusCodeException sub-class has headers and body but unfortunately it accepts an HttpStatus and an HttpStatus cannot be created with an unknown status code. I could create an UnknownStatusCodeException as a parallel class that duplicates the fields of HttpStatusCodeException but uses an int for the status code but it doesn't seem right to do that. Given that there shouldn't be unknown status codes what is the case you actually have run or expect to run into?

Comment by harish [ 14/Jun/12 ]

There are services that that return custom status codes (am running into them currently and have also seen other services using them ), but argueably still adhere to spec. As spec treats 4xx as valid client errors and 5xx as valid server errors (I believe they were meant to be extensible). It can be argued that initially only few say 4xx had defined messages, but they have grown over time.
Having another exception for unknown status codes should also get us there it if we include header body in it.
Alternately, we may also be able to accommodate it without RestClientException class changes if before throwing exception we check status code to be of type 4xx or 5xx and throw HttpClientErrorException and HttpServerErrorException for them and keep RestClientException for other cases. (Services while failing for client related errors have ability to see what client errors are and can pass error messages or details in the body).

Comment by harish [ 18/Jun/12 ]

Including below from rfc 2616: http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

10.4 Client Error 4xx

The 4xx class of status code is intended for cases in which the client seems to have erred. Except when responding to a HEAD request, the server SHOULD include an entity containing an explanation of the error situation, and whether it is a temporary or permanent condition. These status codes are applicable to any request method. User agents SHOULD display any included entity to the user.

10.5 Server Error 5xx

Response status codes beginning with the digit "5" indicate cases in which the server is aware that it has erred or is incapable of performing the request. Except when responding to a HEAD request, the server SHOULD include an entity containing an explanation of the error situation, and whether it is a temporary or permanent condition. User agents SHOULD display any included entity to the user. These response codes are applicable to any request method.

Comment by Rossen Stoyanchev [ 02/Nov/12 ]

I've added an UknownHttpStatusCodeException that carries all the information as the equivalent HttpClientErrorException and HttpServerErrorException exceptions raised when the status code is known. See commit f528c3.

Comment by Spring Issuemaster [ 14/Jan/19 ]

The Spring Framework has migrated to GitHub Issues. This issue corresponds to spring-projects/spring-framework#14042.

Generated at Mon Jan 27 05:48:15 UTC 2020 using Jira 7.13.8#713008-sha1:1606a5c1e7006e1ab135aac81f7a9566b2dbc3a6.