failsafe-lib / failsafe

Fault tolerance and resilience patterns for the JVM
https://failsafe.dev
Apache License 2.0
4.2k stars 297 forks source link

RetryPolicy handleIf not working with DynamoDbException in predicate #370

Closed ntthaibk closed 1 year ago

ntthaibk commented 1 year ago

When I'm doing failsafe with this handleIf statement in the RetryPolicy builder

RetryPolicy.builder()
                .withBackoff(5, maxDelay, ChronoUnit.SECONDS)
                .withMaxRetries(maxRetry)
                .handle(DynamoDbException.class)
                .handleIf(e -> {
                    if (e instanceof IllegalStateException) {
                        return e.getMessage().contains("Record not Found");
                    }
                    if (e instanceof DynamoDbException) {
                        return e.getMessage().contains("Rate exceeded");
                    }
                    return false;
                })
                .onSuccess(e -> log.info("DynamoDb query success"))
                .onFailedAttempt(e -> log.error("No record found with exception {}, attempted: {}", e.getLastException(), e.getAttemptCount()))
                .build();
    }

With other kind of Exception, e.g. IllegalStateException, RuntimeException with custom message, it can match with predicate and handle correctly. But for that DynamoDbException, regardless of the message, it always retry.

I don't know if we can do something to enhance so the predicate can catch the condition e.getMessage().contains("Rate exceeded"); correctly

Thank you guys

Tembrel commented 1 year ago

What happens when you remove the handle(...) line:

    //.handle(DynamoDbException.class)

I'm guessing you're following the model on this page: image

I think that sample code is demonstrating different possibilities rather than being a single model of how to do it.

jhalterman commented 1 year ago

I think that sample code is demonstrating different possibilities rather than being a single model of how to do it.

That's correct.

Tembrel commented 1 year ago

I wonder if there's a simple markdown way to make that clear, e.g.,

policyBuilder
    .handle(ConnectException.class, SocketException.class);
    // OR
    .handleIf(e -> e instanceof ConnectException);

Note the semicolon on both alternatives.

ntthaibk commented 1 year ago

thanks guys, it worked perfectly now.

P/S: as I'm working on it now, if you guys don't mind, I will send an MR to update the docs ^^