Unknown status codes (i.e. not in HttpStatus enum) prevent HttpClientErrorException and HttpServerErrorExceptions from being raised [SPR-9406]

[spring-projects-issues]

harish opened SPR-9406 and commented

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.

---

Affects: 3.1.1

Sub-tasks:

• Backport "Unknown status codes (i.e. not in HttpStatus enum) prevent HttpClientErrorException and HttpServerErrorExceptions from being raised" [SPR-9502] #14136 Backport "Unknown status codes (i.e. not in HttpStatus enum) prevent HttpClientErrorException and HttpServerErrorExceptions from being raised"

Issue Links:

• RestTemplate throws IllegalArgumentException when HTTP status is not in the HttpStatus enum [SPR-6752] #11418 RestTemplate throws IllegalArgumentException when HTTP status is not in the HttpStatus enum
• RestTemplate doesn't consistently tolerate unknown HTTP status codes [SPR-15978] #20529 RestTemplate doesn't consistently tolerate unknown HTTP status codes

[spring-projects-issues]

Rossen Stoyanchev commented

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.

[spring-projects-issues]

harish commented

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()) )

[spring-projects-issues]

harish commented

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.

[spring-projects-issues]

Rossen Stoyanchev commented

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()?

[spring-projects-issues]

Rossen Stoyanchev commented

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

[spring-projects-issues]

harish commented

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

[spring-projects-issues]

Rossen Stoyanchev commented

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.

[spring-projects-issues]

harish commented

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?

[spring-projects-issues]

harish commented

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?

[spring-projects-issues]

Rossen Stoyanchev commented

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?

[spring-projects-issues]

harish commented

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).

[spring-projects-issues]

harish commented

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.

[spring-projects-issues]

Rossen Stoyanchev commented

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.