DefaultRestClient: readWithMessageConverters: retain the statusCode

[puce77]

Affects: 6.1.10

org.springframework.web.client.DefaultRestClient:

If a response cannot be parsed (readWithMessageConverters), a RestClientException (or a subclass) gets thrown instead of the specific subclass RestClientResponseException, making the original responseBody and statusCode very hidden in nested causes.

In such cases a RestClientResponseException should be thrown, so that there is a clean API for getting the statusCode and the original responseBody.

Common base class for exceptions that contain actual HTTP response data.

spring-framework/spring-web/src/main/java/org/springframework/web/client/DefaultRestClient.java

Line 202 in 4a099d2

private <T> T readWithMessageConverters(ClientHttpResponse clientResponse, Runnable callback, Type bodyType,

[bclozel]

So far, RestClientResponseException has been used for specific cases like HTTP responses with status > 400.
Expanding RestClientResponseException could have significant repercussions as the body is buffered entirely in memory as a byte[], whereas the HttpMessageNotReadableException thrown by converters contains the HttpInputMessage with the body as input stream. We would need to buffer the entire body into a byte array to make this work, which could consume a lot of memory if the response body is large. Some might even consider this a security issue.

Is that what you had in mind?

[puce77]

Currently I'm mainly interesseted in the statusCode. If we get a statusCode and it is in the 2xx-family, we know the remote service could process everything, we "just" could not parse the response (deserialization step).
There are a couple of scenarios where there is actually really no statusCode (connect timeout, message serialization exception during writing the InputStream etc.). In those cases RestClientException is fine as there is no status code to report. But detecting those different cases is important for proper error handling.

I guess the byte[] responseBody in the RestClientResponseException class usually holds smaller (error) messages and I guess in the case we are talking about you could get the original message from the log output for analyzing purposes. So I guess we could omit that for now, if you think that is an issue.

But to get the statusCode I need currently to do something like this:

        } catch (RestClientException rce) {
            if (rce.getCause() instanceof HttpMessageNotReadableException hmnre) {
                if (hmnre.getHttpInputMessage() instanceof ClientHttpResponse clientHttpResponse) {
                    try {
                        HttpStatusCode statusCode = clientHttpResponse.getStatusCode();
                    } catch (IOException e) {
                        // ...
                    }
                }
            }

And this looks quite brittle.

[puce77]

Interestingly, the WebClient API throws in such cases a DecodingException, which is not a subclass of WebClientException. You still don't know the exact statusCode, but since you can clearly distinguish it from the other errors, you at least know there must have been a statusCode in the 2xx-family.

[puce77]

Also note: if no message converter is found, an UnknownContentTypeException gets thrown. This class contains the statusCode as well as the responseBody (!) but extends RestClientException not RestClientResponseException .

The WebClient API throws a WebClientResponseException with statusCode but with an empty (!) responseBody in such cases (the cause is UnsupportedMediaTypeException).

[rstoyanchev]

The problem with an HttpMessageNotReadableException is that some of the content may already have been read, e.g. JSON parsing error half way through, and in that case we can't create RestClientResponseException because it expects the body and while that's nullable, "no body" is not the same as "body that can't be exposed" because it's partly consumed.

To put it differently, RestClientException vs RestClientResponseException separates requests that went wrong before we got a full response. An IOException or HttpMessageNotReadableException as the cause further indicates the content couldn't be read. I'm not sure if the exact status really matters in that case. It's definitely something in the success range.

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.