Retry with OpenFeign Add-On not working about resilience4j HOT 8 OPEN

Hi
Can I work on this bug, please assign to me
Thanks

from resilience4j.

Done :D

from resilience4j.

Hi, I tried to reproduce, but it looks to be working fine for me.

I could see that Resilience retry is getting used to invoke feign client.
I was also see at server logs, the number of retries ( I tired 4 and then 2) were also as per resilience 4j retry config

Here is the code:

@SpringBootTest
public class ResTests {

@Test()
public void testRes() {
	
			
		Retry retry = Retry.of("test",RetryConfig.custom().retryExceptions(FeignException.class).maxAttempts(4).build());
		 final FeignDecorators decorators = FeignDecorators.builder()
		            .withRetry(retry)
		            .build();
		 
		 BackendService testService = Resilience4jFeign.builder(decorators)
		            .target(BackendService.class, "http://localhost:8080");

		 testService.getData();
		 
		 
}

}

the stack trace at unit test I get shows resilience 4j invoker:

feign.FeignException$NotFound: [404] during [GET] to [http://localhost:8080/ex3/foos] [BackendService#getData()]: [{"timestamp":"2024-01-19T05:43:51.929+00:00","status":404,"error":"Not Found","path":"/ex3/foos"}]
at feign.FeignException.clientErrorStatus(FeignException.java:219)
at feign.FeignException.errorStatus(FeignException.java:194)
at feign.FeignException.errorStatus(FeignException.java:185)
at feign.codec.ErrorDecoder$Default.decode(ErrorDecoder.java:92)
at feign.AsyncResponseHandler.handleResponse(AsyncResponseHandler.java:96)
at feign.SynchronousMethodHandler.executeAndDecode(SynchronousMethodHandler.java:138)
at feign.SynchronousMethodHandler.invoke(SynchronousMethodHandler.java:89)
at io.github.resilience4j.retry.Retry.lambda$decorateCheckedFunction$7bb28b04$1(Retry.java:187)
at io.github.resilience4j.feign.DecoratorInvocationHandler.invoke(DecoratorInvocationHandler.java:95)
at jdk.proxy2/jdk.proxy2.$Proxy118.getData(Unknown Source)
at com.amit.test.demo.resbug.ResTests.testRes(ResTests.java:39)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Also the since I did not mock the webservice call, hence I was able to see server access logs where I could see retried URLS as per number configured in retry config.
Hence It is not an issue.

from resilience4j.

Hi!

I investigated a bit more, here is what i found out:

I cloned https://github.com/resilience4j/resilience4j/tree/v2.2.0 and altered io.github.resilience4j.feign.Resilience4jFeignRetryTest#testFailedHttpCallWithRetry from original:

    @Test(expected = FeignException.class)
    public void testFailedHttpCallWithRetry() {
        retry = Retry.of("test",RetryConfig.custom().retryExceptions(FeignException.class).maxAttempts(2).build());
        final FeignDecorators decorators = FeignDecorators.builder()
            .withRetry(retry)
            .build();
        testService = Resilience4jFeign.builder(decorators)
            .target(TestService.class, MOCK_URL);
        givenResponse(400);
        testService.greeting();
        assertThat(retry.getMetrics().getNumberOfFailedCallsWithRetryAttempt()).isEqualTo(2);
    }

    @Test
    public void testFailedHttpCallWithRetry() {
        retry = Retry.of("test",RetryConfig.custom().retryExceptions(FeignException.class).maxAttempts(2).build());
        final FeignDecorators decorators = FeignDecorators.builder()
            .withRetry(retry)
            .build();
        testService = Resilience4jFeign.builder(decorators)
            .target(TestService.class, MOCK_URL);
        givenResponse(400);
        try {
            testService.greeting();
        } catch (Throwable e) {
            assertThat(e).isInstanceOf(FeignException.class);
        }
        verify(2, getRequestedFor(urlEqualTo("/greeting")));
        assertThat(retry.getMetrics().getNumberOfFailedCallsWithRetryAttempt()).isEqualTo(1);

        try {
            testService.greeting();
        } catch (Throwable e) {
            assertThat(e).isInstanceOf(FeignException.class);
        }
        verify(4, getRequestedFor(urlEqualTo("/greeting")));
        assertThat(retry.getMetrics().getNumberOfFailedCallsWithRetryAttempt()).isEqualTo(2);
    }

My finding is, that what @amitarcade said is true. The originally implementation of the test made me believe that retry.getMetrics().getNumberOfFailedCallsWithRetryAttempt() returns the number of actual calls, but it does not. Anyway, this is a problem, that is not relevant for Feign but for Resilience4j.

However, i tried to reproduce my problem and the following test shows it:

    @Test
    public void testSlowHttpCallWithRetry() {
        final Request.Options feignOptions = new Request.Options(1L, TimeUnit.SECONDS, 1L, TimeUnit.SECONDS, true);
        retry = Retry.of("test",RetryConfig.custom().retryExceptions(FeignException.class).maxAttempts(2).build());
        final FeignDecorators decorators = FeignDecorators.builder()
                .withRetry(retry)
                .build();
        testService = Resilience4jFeign.builder(decorators).options(feignOptions)
                .target(TestService.class, MOCK_URL);
        stubFor(get(urlPathEqualTo("/greeting"))
                .willReturn(aResponse()
                        .withFixedDelay(3000)
                        .withStatus(200)
                        .withHeader("Content-Type", "text/plain")
                        .withBody("hello world")));
        try {
            testService.greeting();
        } catch (Throwable e) {
            assertThat(e).isInstanceOf(RetryableException.class);
        }
        verify(2, getRequestedFor(urlEqualTo("/greeting")));
        assertThat(retry.getMetrics().getNumberOfFailedCallsWithRetryAttempt()).isEqualTo(1);
    }

In this test i do force the Feign Client to fail fast (1 second timeouts). I expect Resilience4j and Feign to retry 1 more time, but it does 10 times:

com.github.tomakehurst.wiremock.client.VerificationException: Expected exactly 2 requests matching the following pattern but received 10

It looks like the Resilience4j and Feign retries do multiply. @RobWin is this expected behavior?
Adding the following configuration (manually disable Feign Retryer) results in 2 requests:

final Retryer retryer = new Retryer.Default(100L, 1_000L, 0);
...
testService = Resilience4jFeign.builder(decorators).options(feignOptions).retryer(retryer)
                .target(TestService.class, MOCK_URL);

from resilience4j.

There can be issue with Wiremock, please check
wiremock/wiremock#1789
Maybe try ApacheClient as an alternative

from resilience4j.

As others in the issue mentioned, i dont think this is Wiremock related. The HTTP Client might be initializing retries, but thats exactly what i want to configure. :)

There is definitely a correlation between RetryConfig#maxAttempts and Retryer.Default#maxAttempts. The actual number of calls is always the multiplication of both values.

For me, this is unexpected but it might be desired for a reason i do not get.

from resilience4j.

Consider a scenario where you employ an HTTP client with an inherent retry mechanism that automatically retries three times upon encountering a 500 status code, subsequently resulting in a thrown HttpServerErrorException. Simultaneously, you integrate Resilience4j Retry. In the absence of explicitly ignoring the HttpServerErrorException, the HTTP calls undergo additional retries through Resilience4j Retry. Consequently, this leads to a compounding effect on the overall retry attempts.

Is that the scenario what you are currently facing with Feign, but with 404 status code?

from resilience4j.

Almost.

The HTTP Client throws a java.net.SocketTimeoutException: Read timed out (wrapped within a RetryableException), because the configured "read timeout" is exceeded. This seems to lead to cascading calls of both retry implementations thus in compounding retries.

Valid calls (+response without timeout) with status code 500 (did not check 404) act differently. Seemingly Feign's retry implementation is not triggered.

from resilience4j.