Include response body in RestTemplate exception when there is no suitable HttpMessageConverter

[devinabyss]

When RestTemplate receive Response from target server, but there's no body readerble converter, throws RestClientException with only situation describing message.

In this situation, even though communication with the server is normally completed, the content of the received text is lost only because the object conversion of the specified type is impossible.

The results seem unfriendly to developers.

It is extremely natural that Exception is thrown because object conversion is impossible, but preserving the response value so that the developer can confirm that the object was not converted to the intended object because the value was returned in the response is helpful in the development or production stage. I think it need. (Especially production)

sually, if the values of the object conversion failure case are unexpectedly returned in the body, the status code may not 2xx, so an HttpServerErrorResponse or HttpClientErrorException will be thrown, but there is no 100% guarantee that the other server will follow the general status code rules. It may or may not be intentional.

Also, may have misplaced the conversion specification simply by the carelessness of the developer writing the communication code.

Anyway, the important thing is that it's a good idea to provide more effective information for trace exception by providing the full value of the response body returned to the developer.

I searched similar issue on github and stackoverflow, but could not find.

So I modified codes of HttpMessageConverterExtractor.class near line 120 as below, and I'd like to hear your opinion.

I would appreciate it if you would like to keep the body values or if there is a better way.
thanks.

Current

		...
		catch (IOException | HttpMessageNotReadableException ex) {
			throw new RestClientException("Error while extracting response for type [" +
					this.responseType + "] and content type [" + contentType + "]", ex);
		}

		throw new RestClientException("Could not extract response: no suitable HttpMessageConverter found " +
				"for response type [" + this.responseType + "] and content type [" + contentType + "]");
	}
	...

Suggest

		...
		} 
		catch (IOException | HttpMessageNotReadableException ex) {
			throw new RestClientException("Error while extracting response for type [" +
					this.responseType + "] and content type [" + contentType + "]", ex);
		}

		String extractErrorMessage = "Could not extract response: no suitable HttpMessageConverter found " +
				"for response type [" + this.responseType + "] and content type [" + contentType + "]";

		try (InputStream is = responseWrapper.getBody()) {
			byte[] bytes = StreamUtils.copyToByteArray(is);
			throw new RestClientResponseException(extractErrorMessage, response.getStatusCode().value(),
					response.getStatusText(), response.getHeaders(), bytes, StandardCharsets.UTF_8);
		} catch (IOException ex) {
			throw new RestClientException(extractErrorMessage + " And Error while keep response body to bytes", ex);
		}
	}
	...

[rstoyanchev]

This is expected behavior. IOException or HttpMessageNotReadableException are problems while reading the response body which can only be read once. This is why the code here raises a plain RestClientException. By contrast for 4xx and 5xx we raise RestClientResponseException (or one of its sub-classes) with the body.

[devinabyss]

@rstoyanchev

I want keep response body only theres no suitable converter exist case.

in this case, response body not read why in message converter check loop execute first canRead converters method with class type check and return false without read stream.

not when exeception ariesed while response body stream read. (IOException or HttpMessageNotReadableException)

like this case..

    @Test
    public void test(){
        try {
            Map body = restTemplate.getForObject("https://www.google.com/", Map.class);
        }catch (RestClientException e) {
            // current status just only throws RestClientException without body end search of for loop. without stream read.
            log.info("{}, {}", e.getClass().getCanonicalName(), e.getMessage());

            // when modified, developer can check response body
            if (e instanceof RestClientResponseException){
                log.info("## No Suitable MessageConverter Body ##");
                log.info("{}", ((RestClientResponseException) e).getResponseBodyAsString());
             }
        }
    }

log..

16:18:15.062 [main] DEBUG org.springframework.web.client.RestTemplate - HTTP GET https://www.google.com/
16:18:15.067 [main] DEBUG org.springframework.web.client.RestTemplate - Accept=[]
16:18:15.490 [main] DEBUG org.springframework.web.client.RestTemplate - Response 200 OK
16:18:15.546 [main] INFO just.RestTemplateTest - org.springframework.web.client.RestClientResponseException, Could not extract response: no suitable HttpMessageConverter found for response type [interface java.util.Map] and content type [text/html;charset=ISO-8859-1]
16:18:15.547 [main] INFO just.RestTemplateTest - ## No Suitable MessageConverter Body ##
16:18:15.547 [main] INFO just.RestTemplateTest - <!doctype html><html itemscope="" itemtype="http://schema.org/WebPage" lang="ko"><head><meta content="text/html; charset=UTF-8" http-equiv="Content-Type"><meta content="/images/branding/googleg/1x/googleg_standard_color_128dp.png" itemprop="image"><title>Google</title><script nonce="LnGeMSOMQPj+5Hkhy7F9Ww==">(function(){window.google={kEI:'t5KiXvSjPNPEmAWXjrHoCw',kEXPI:'31',

[rstoyanchev]

Yes I see now.

@poutsma do you think using RestClientResponseException, since it allows including the response content, be okay or would that only be expected for 4xx and 5xx responses?

[devinabyss]

thanks!

when detail spec discussion end, like RestClientResponseException use

can i create pull request base on that code?

[rstoyanchev]

can i create pull request base on that code?

Yes.

[poutsma]

@poutsma do you think using RestClientResponseException, since it allows including the response content, be okay or would that only be expected for 4xx and 5xx responses?

Yes, I think RestClientResponseException suffices, but I would prefer it if we introduce a new subclass for this purpose, rather than using RestClientResponseException itself.

[verils]

Why not just keep the modification of RestClientResponseException.FailedToReadResponseBody, or switch to another specific Exception?

I have same situation when I got a HttpMessageNotReadableException. For now, I do not concern what content data the server responds, but I do care about what status code is. It's neccessary containing the status code in raised exception.

If needed I can submit a PR to resolve this.