Retry issue after migration to 13.5

[sergey-morenets]

Hi

We use feign-java11 and feign-gson dependencies in our project and after we migrated from 13.1 to 13.5 one of our tests started failing.
This test uses WireMock and verifies that if malformed response is sent to the client then RetryableException is thrown.

extension.stubFor(post(urlEqualTo("/cities")).willReturn(aResponse().withFault(Fault.MALFORMED_RESPONSE_CHUNK)));
assertThrows(RetryableException.class, () -> cityClient.create(city));

In 13.1 this test passed but in 13.5 fails. Our investigation shown that in both Feign versions IOException is thrown but in 13.1 it's correctly caught in SynchronousMethodHandler.executeAndDecode:

java.io.IOException: chunked transfer encoding, state: READING_LENGTH

And in 13.5 it's not caught. The last line that was executed is

cf = sendAsync(req, responseHandler, null, null);

in HttpClientImpl.send

[sergey-morenets]

Our deeper analysis shown that this regression appeared in 13.3 and possible reason is the replacement

httpResponse = clientForRequest.send(httpRequest, BodyHandlers.ofByteArray());

with

httpResponse = clientForRequest.send(httpRequest, BodyHandlers.ofInputStream());

in Http2Client.

[sergey-morenets]

So IOException actually happens then we try to read response body (in IOUtils.toString):

var response = cityClient.create(city);
var body = IOUtils.toString(response.body().asInputStream(), Charset.defaultCharset());

And main issue is that this exception occurs outside of Feign client and that means retry logic would not be executed.

[hdfg159]

If cityClient.create(city) returns a feign.Response, it should indeed handle the inputStream related exceptions by itself.

If you want to reproduce the previous logic, the return type should be filled with byte[].
eg: cityClient

byte[] create(var city)

[sergey-morenets]

Hi @hdfg159

I'm fine with that but it's breaking change and should be mentioned in release notes and documentation.

[hdfg159]

Hi @hdfg159

I'm fine with that but it's breaking change and should be mentioned in release notes and documentation.

This should be a bug -> #2419 . If it's version 13.1, then using other clients(eg: apache httpclient or okhttp3) should yield different expected results.

[hdfg159]

If cityClient.create(city) returns a feign.Response, it should indeed handle the inputStream related exceptions by itself.

If you want to reproduce the previous logic, the return type should be filled with byte[]. eg: cityClient

byte[] create(var city)

#1474 (comment)