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

Retry policy doesn't work properly if it contains `.handleResult(null)` #240

Closed stsdema28 closed 4 years ago

stsdema28 commented 4 years ago

The following code demonstrates the issue that happens when a retry policy contains .handleResult(null):

public class Issue {

    static String doSomething() {
        throw new NullPointerException();
    }

    public static void main(String[] args) {
        RetryPolicy<Object> retryPolicy = new RetryPolicy<>()
                .handle(IllegalArgumentException.class)
                .handleResult(null)
                .withMaxRetries(2)
                .onRetry(e -> System.out.println("retry attempt: " + e.getAttemptCount()));

        try {
            Failsafe.with(retryPolicy).get(Issue::doSomething);
        } catch (NullPointerException e) {
            System.out.println("It should not retry on NullPointerException");
        }
    }
}

The above prints:

retry attempt: 1
retry attempt: 2
It should not retry on NullPointerException

which means that it retried due to NullPointerException.

If we remove .handleResult(null), it prints:

It should not retry on NullPointerException

which is the correct behaviour as it should retry only on IllegalArgumentException.class.

Just to note, the issue is only with null. If we specify any other value e.g .handleResult("hello world"), there is no problem.

jhalterman commented 4 years ago

Fixed in 73019c0aabf1ca6d2229748e3dc797a74d8f4cf0.

jhalterman commented 4 years ago

This is released in 2.3.4.