AsyncRestTemplate should wrap RuntimeExceptions in ExecutionException [SPR-13413] #17992
Description
Lukas Krecan opened SPR-13413 and commented
We are migrating to RestTemplate and for backward compatibility reasons we have custom ResponseErrorHandler that throws RuntimExceptions. AsyncRestTemplate uses ResponseExtractorFuture which wraps IOExceptions to ExecutionException. Unfortunately it does not do the same for runtime exceptions which in turn causes different problems (for example response "caching" in ListenableFutureAdapters does not work)
It concerns this code from ResponseExtractorFuture (see comments inside)
@Override
protected final T adapt(ClientHttpResponse response) throws ExecutionException {
try {
if (!getErrorHandler().hasError(response)) {
logResponseStatus(this.method, this.url, response);
}
else {
handleResponseError(this.method, this.url, response); // throws Runtime exception
}
return convertResponse(response);
}
catch (IOException ex) { // here I suggest to catch RuntimeException as well
throw new ExecutionException(ex);
}
finally {
if (response != null) {
response.close();
}
}
}
Affects: 4.1.6
Issue Links:
- HttpClientErrorException missing response body if callback is registered [SPR-12887] #17485 HttpClientErrorException missing response body if callback is registered
- AsyncRestTemplate changes in SPR-13413 made its harder to detect Client/Server errors [SPR-14353] #18925 AsyncRestTemplate changes in SPR-13413 made its harder to detect Client/Server errors