Fallback-Handling changes from version 2.3.4 to 2.4.0

[mcjr]

With version 2.3.4 the following functionality calls only the Fallback notFoundFallback if a ConnectException is thrown in connect(). But in version 2.4.0 the aspect calls both Fallbacks: notFoundFallback and failureHandling.

import net.jodah.failsafe.*;
import net.jodah.failsafe.event.ExecutionAttemptedEvent;
import net.jodah.failsafe.function.CheckedFunction;

import java.net.ConnectException;
import java.time.Duration;

public class Main {
    public static void main(String[] args) {
        new Main().execute();
    }

    private void execute() {
        Timeout<Object> timeout = Timeout.of(Duration.ofMillis(1000L));
        Fallback<Object> notFoundFallback = Fallback.of((CheckedFunction<ExecutionAttemptedEvent<?>, ?>) this::handleNotFound).handleIf(this::causedBy404);
        CircuitBreaker<Object> circuitBreaker = new CircuitBreaker<>();
        Fallback<Object> failureHandling = Fallback.ofException(this::handleException);

        FailsafeExecutor<Object> executor = Failsafe.with(failureHandling, circuitBreaker, notFoundFallback, timeout);
        Integer result = executor.get(this::connect);
        System.out.println("result = " + result);
    }

    private Integer connect() throws ConnectException {
            throw new ConnectException();
    }

    private boolean causedBy404(Object o, Throwable throwable) {
        return throwable instanceof ConnectException;
    }

    private Object handleNotFound(ExecutionAttemptedEvent<?> event) {
        System.out.println("handleNotFound");
        return null;
    }

    private Exception handleException(ExecutionAttemptedEvent<?> event) {
        System.out.println("handleException");
        return new IllegalArgumentException();
    }
}

Therefore, with the version 2.3.4 there is the output:

handleNotFound
result = null

However, with version 2.4.0 there is:

handleNotFound
handleException
Exception in thread "main" java.lang.IllegalArgumentException
	at Main.handleException(Main.java:39)
	at net.jodah.failsafe.Fallback.lambda$ofException$0(Fallback.java:119)
	at net.jodah.failsafe.Fallback.apply(Fallback.java:235)
	at net.jodah.failsafe.FallbackExecutor.lambda$supply$0(FallbackExecutor.java:48)
	at net.jodah.failsafe.Execution.executeSync(Execution.java:129)
	at net.jodah.failsafe.FailsafeExecutor.call(FailsafeExecutor.java:376)
	at net.jodah.failsafe.FailsafeExecutor.get(FailsafeExecutor.java:67)
	at Main.execute(Main.java:20)
	at Main.main(Main.java:10)

Is this a bug? Or is there a method to get the old behavior back?

[Tembrel]

I think it's a bug.

First of all, I think the circuitBreaker and the outer Fallback are red herrings. I get the same results if I omit the circuit breaker from the Failsafe.with call, and the confusing stack trace seems to be the result of throwing IAE when handling the ConnectException.

Note that if you return a non-null value, e.g., 404, from handleNotFound, it works as you expect in both 2.3.4 and 2.4.0:

handleNotFound
result = 404

So apparently a null result value for a Fallback is seen as a failure. That doesn't fit the spec: Fallback.of(R result) is not documented as throwing NPE, but specifying a null return value (whether by Fallback.of((Object)null) or Fallback.of(e -> null)) doesn't result in a null return value from the execution.

That explains why the fallback doesn't do its job: When it re-evaluates its handle condition with its fallback result, the same arguments (null, ConnectException) are passed to causedBy404 again, and this time it fails for good. If Fallback did not have to re-evaluate the handle condition, this would work as expected. See #248 for long discussion about this.

[jhalterman]

Thanks for reporting this. This will be fixed in the next release.

[jhalterman]

This is fixed in 2.4.2.