Merge pull request #1008 from Ladicek/improve-cause-chain-inspection

improve cause chain inspection

doc/modules/ROOT/pages/reference/circuit-breaker.adoc

@@ -242,34 +242,41 @@ include::partial$non-compat.adoc[]
 
 The `@CircuitBreaker` annotation can specify that certain exceptions should be treated as failures (`failOn`) and others as successes (`skipOn`).
 The specification limits this to inspecting the actual exception that was thrown.
-However, in many usecases, exceptions are wrapped and the exception the user wants to decide on is only present in the cause chain.
+However, in many cases, exceptions are wrapped and the exception the user wants to decide on is only present in the cause chain.
 
 For that reason, in the non-compatible mode, if the actual thrown exception isn't known failure or known success, {smallrye-fault-tolerance} inspects the cause chain.
 To be specific, in case a `@CircuitBreaker` method throws an exception, the decision process is:
 
-1. If the exception is assignable to one of the `skipOn` exceptions, the circuit breaker treats it as a success.
-2. Otherwise, if the exception is assignable to one of the `failOn` exceptions, the circuit breaker treats it as a failure.
-3. Otherwise, if the cause chain of the exception contains an exception assignable to one of the `skipOn` exceptions, the circuit breaker treats it as a success.
-4. Otherwise, if the cause chain of the exception contains an exception assignable to one of the `failOn` exceptions, the circuit breaker treats it as a failure.
+1. If the `skipOn` exceptions are not default and the exception is assignable to one of the `skipOn` exceptions, the circuit breaker treats it as a success.
+2. Otherwise, if the `failOn` exceptions are not default and the exception is assignable to one of the `failOn` exceptions, the circuit breaker treats it as a failure.
+3. Otherwise, if the exception is assignable to one of the `skipOn` exceptions or its cause chain contains an exception assignable to one of the `skipOn` exceptions, the circuit breaker treats it as a success.
+4. Otherwise, if the exception is assignable to one of the `failOn` exceptions or its cause chain contains an exception assignable to one of the `failOn` exceptions, the circuit breaker treats it as a failure.
 5. Otherwise, the exception is treated as a success.
 
-For example, say we have this method:
+For example:
 
 [source,java]
 ----
 @CircuitBreaker(requestVolumeThreshold = 10,
-    skipOn = ExpectedOutcomeException.class,
-    failOn = IOException.class)
+    skipOn = ExpectedOutcomeException.class, // <1>
+    failOn = IOException.class) // <2>
 public Result doSomething() {
     ...
 }
 ----
 
-If `doSomething` throws an `ExpectedOutcomeException`, the circuit breaker treats it as a success.
-If `doSomething` throws an `IOException`, the circuit breaker treats it as a failure.
-If `doSomething` throws a `WrapperException` whose cause is `ExpectedOutcomeException`, the circuit breaker treats it as a success.
-If `doSomething` throws a `WrapperException` whose cause is `IOException`, the circuit breaker treats it as a failure.
+<1> If `doSomething` throws an `ExpectedOutcomeException`, or a `WrapperException` whose cause is `ExpectedOutcomeException`, the circuit breaker treats it as a success.
+<2> If `doSomething` throws an `IOException`, or a `WrapperException` whose cause is `IOException`, the circuit breaker treats it as a failure.
 
-Comparing with the `@CircuitBreaker` specification, {smallrye-fault-tolerance} inserts 2 more steps into the decision process that inspect the cause chain.
-Note that these steps are executed if and only if the thrown exception matches neither `failOn` nor `skipOn`.
-If the thrown exception matches either of them, the cause chain is not inspected at all.
+[source,java]
+----
+@CircuitBreaker(requestVolumeThreshold = 10,
+    skipOn = ExpectedOutcomeException.class) // <1> <2>
+public Result doSomething() {
+    ...
+}
+----
+
+<1> If `doSomething` throws an `ExpectedOutcomeException`, or a `WrapperException` whose cause is `ExpectedOutcomeException`, the circuit breaker treats it as a success.
+<2> There's no `failOn`, so the 2nd step in the algorithm above is skipped.
+This is what turns the `WrapperException` whose cause is `ExpectedOutcomeException` into a success.

doc/modules/ROOT/pages/reference/fallback.adoc

@@ -252,24 +252,24 @@ include::partial$non-compat.adoc[]
 
 The `@Fallback` annotation can specify that certain exceptions should be treated as failures (`applyOn`) and others as successes (`skipOn`).
 The specification limits this to inspecting the actual exception that was thrown.
-However, in many usecases, exceptions are wrapped and the exception the user wants to decide on is only present in the cause chain.
+However, in many cases, exceptions are wrapped and the exception the user wants to decide on is only present in the cause chain.
 
 For that reason, in the non-compatible mode, if the actual thrown exception isn't known failure or known success, {smallrye-fault-tolerance} inspects the cause chain.
 To be specific, in case a `@Fallback` method throws an exception, the decision process is:
 
-1. If the exception is assignable to one of the `skipOn` exceptions, fallback is skipped and the exception is rethrown.
-2. Otherwise, if the exception is assignable to one of the `applyOn` exceptions, fallback is applied.
-3. Otherwise, if the cause chain of the exception contains an exception assignable to one of the `skipOn` exceptions, fallback is skipped and the exception is rethrown.
-4. Otherwise, if the cause chain of the exception contains an exception assignable to one of the `applyOn` exceptions, fallback is applied.
+1. If the `skipOn` exceptions are not default and the exception is assignable to one of the `skipOn` exceptions, fallback is skipped and the exception is rethrown.
+2. Otherwise, if the `applyOn` exceptions are not default and the exception is assignable to one of the `applyOn` exceptions, fallback is applied.
+3. Otherwise, if the exception is assignable to one of the `skipOn` exceptions or its cause chain contains an exception assignable to one of the `skipOn` exceptions, fallback is skipped and the exception is rethrown.
+4. Otherwise, if the exception is assignable to one of the `applyOn` exceptions or its cause chain contains an exception assignable to one of the `applyOn` exceptions, fallback is applied.
 5. Otherwise, the exception is rethrown.
 
-For example, say we have this method:
+For example:
 
 [source,java]
 ----
 @Fallback(fallbackMethod = "fallback",
-    skipOn = ExpectedOutcomeException.class,
-    applyOn = IOException.class)
+    skipOn = ExpectedOutcomeException.class, // <1>
+    applyOn = IOException.class) // <2>
 public Result doSomething() {
     ...
 }
@@ -279,11 +279,22 @@ public Result fallback() {
 }
 ----
 
-If `doSomething` throws an `ExpectedOutcomeException`, fallback is skipped and the exception is thrown.
-If `doSomething` throws an `IOException`, fallback is applied.
-If `doSomething` throws a `WrapperException` whose cause is `ExpectedOutcomeException`, fallback is skipped and the exception is thrown.
-If `doSomething` throws a `WrapperException` whose cause is `IOException`, fallback is applied.
+<1> If `doSomething` throws an `ExpectedOutcomeException`, or a `WrapperException` whose cause is `ExpectedOutcomeException`, fallback is skipped and the exception is thrown.
+<2> If `doSomething` throws an `IOException`, or a `WrapperException` whose cause is `IOException`, fallback is applied.
+
+[source,java]
+----
+@Fallback(fallbackMethod = "fallback",
+    skipOn = ExpectedOutcomeException.class) // <1> <2>
+public Result doSomething() {
+    ...
+}
+
+public Result fallback() {
+    ...
+}
+----
 
-Comparing with the `@Fallback` specification, {smallrye-fault-tolerance} inserts 2 more steps into the decision process that inspect the cause chain.
-Note that these steps are executed if and only if the thrown exception matches neither `skipOn` nor `applyOn`.
-If the thrown exception matches either of them, the cause chain is not inspected at all.
+<1> If `doSomething` throws an `ExpectedOutcomeException`, or a `WrapperException` whose cause is `ExpectedOutcomeException`, fallback is skipped and the exception is thrown.
+<2> There's no `applyOn`, so the 2nd step in the algorithm above is skipped.
+This is what turns the `WrapperException` whose cause is `ExpectedOutcomeException` into a skipped fallback.

doc/modules/ROOT/pages/reference/retry.adoc

@@ -245,34 +245,41 @@ include::partial$non-compat.adoc[]
 
 The `@Retry` annotation can specify that certain exceptions should be treated as failures (`retryOn`) and others as successes (`abortOn`).
 The specification limits this to inspecting the actual exception that was thrown.
-However, in many usecases, exceptions are wrapped and the exception the user wants to decide on is only present in the cause chain.
+However, in many cases, exceptions are wrapped and the exception the user wants to decide on is only present in the cause chain.
 
 For that reason, in the non-compatible mode, if the actual thrown exception isn't known failure or known success, {smallrye-fault-tolerance} inspects the cause chain.
 To be specific, in case a `@Retry` method throws an exception, the decision process is:
 
-1. If the exception is assignable to one of the `abortOn` exceptions, retry is aborted and the exception is rethrown.
-2. Otherwise, if the exception is assignable to one of the `retryOn` exceptions, retry is attempted.
-3. Otherwise, if the cause chain of the exception contains an exception assignable to one of the `abortOn` exceptions, retry is aborted and the exception is rethrown.
-4. Otherwise, if the cause chain of the exception contains an exception assignable to one of the `retryOn` exceptions, retry is attempted.
+1. If the `abortOn` exceptions are not default and the exception is assignable to one of the `abortOn` exceptions, retry is aborted and the exception is rethrown.
+2. Otherwise, if the `retryOn` exceptions are not default and the exception is assignable to one of the `retryOn` exceptions, retry is attempted.
+3. Otherwise, if the exception is assignable to one of the `abortOn` exceptions or its cause chain contains an exception assignable to one of the `abortOn` exceptions, retry is aborted and the exception is rethrown.
+4. Otherwise, if the exception is assignable to one of the `retryOn` exceptions or its cause chain contains an exception assignable to one of the `retryOn` exceptions, retry is attempted.
 5. Otherwise, the exception is rethrown.
 
-For example, say we have this method:
+For example:
 
 [source,java]
 ----
 @Retry(maxRetries = 5,
-    abortOn = ExpectedOutcomeException.class,
-    retryOn = IOException.class)
+    abortOn = ExpectedOutcomeException.class, // <1>
+    retryOn = IOException.class) // <2>
 public Result doSomething() {
     ...
 }
 ----
 
-If `doSomething` throws an `ExpectedOutcomeException`, retry is aborted and the exception is thrown.
-If `doSomething` throws an `IOException`, retry is attempted.
-If `doSomething` throws a `WrapperException` whose cause is `ExpectedOutcomeException`, retry is aborted and the exception is thrown.
-If `doSomething` throws a `WrapperException` whose cause is `IOException`, retry is attempted.
+<1> If `doSomething` throws an `ExpectedOutcomeException`, or a `WrapperException` whose cause is `ExpectedOutcomeException`, retry is aborted and the exception is thrown.
+<2> If `doSomething` throws an `IOException`, or a `WrapperException` whose cause is `IOException`, retry is attempted.
+
+[source,java]
+----
+@Retry(maxRetries = 5,
+    abortOn = ExpectedOutcomeException.class) // <1> <2>
+public Result doSomething() {
+    ...
+}
+----
 
-Comparing with the `@Retry` specification, {smallrye-fault-tolerance} inserts 2 more steps into the decision process that inspect the cause chain.
-Note that these steps are executed if and only if the thrown exception matches neither `abortOn` nor `retryOn`.
-If the thrown exception matches either of them, the cause chain is not inspected at all.
+<1> If `doSomething` throws an `ExpectedOutcomeException`, or a `WrapperException` whose cause is `ExpectedOutcomeException`, retry is aborted and the exception is thrown.
+<2> There's no `retryOn`, so the 2nd step in the algorithm above is skipped.
+This is what turns the `WrapperException` whose cause is `ExpectedOutcomeException` into an aborted retry.

implementation/core/src/main/java/io/smallrye/faulttolerance/core/util/SetBasedExceptionDecision.java

@@ -14,35 +14,55 @@ public class SetBasedExceptionDecision implements ExceptionDecision {
 
     private final boolean inspectCauseChain;
 
+    private final boolean nonDefaultConsideredFailure;
+    private final boolean nonDefaultConsideredExpected;
+
     public SetBasedExceptionDecision(SetOfThrowables consideredFailure, SetOfThrowables consideredExpected,
             boolean inspectCauseChain) {
         this.consideredFailure = checkNotNull(consideredFailure, "Set of considered-failure throwables must be set");
         this.consideredExpected = checkNotNull(consideredExpected, "Set of considered-expected throwables must be set");
         this.inspectCauseChain = inspectCauseChain;
+        this.nonDefaultConsideredFailure = !consideredFailure.isAll();
+        this.nonDefaultConsideredExpected = !consideredExpected.isEmpty();
     }
 
     public boolean isConsideredExpected(Throwable e) {
         // per `@CircuitBreaker` javadoc, `skipOn` takes priority over `failOn`
         // per `@Fallback` javadoc, `skipOn` takes priority over `applyOn`
         // per `@Retry` javadoc, `abortOn` takes priority over `retryOn`
         // to sum up, the exceptions considered expected win over those considered failure
+        if (inspectCauseChain) {
+            return isConsideredExpectedWithCauseChain(e);
+        } else {
+            return isConsideredExpectedDefault(e);
+        }
+    }
 
+    private boolean isConsideredExpectedDefault(Throwable e) {
         if (consideredExpected.includes(e.getClass())) {
             return true;
         }
         if (consideredFailure.includes(e.getClass())) {
             return false;
         }
-        if (!inspectCauseChain) {
+        return true;
+    }
+
+    private boolean isConsideredExpectedWithCauseChain(Throwable e) {
+        if (nonDefaultConsideredExpected && consideredExpected.includes(e.getClass())) {
             return true;
         }
+        if (nonDefaultConsideredFailure && consideredFailure.includes(e.getClass())) {
+            return false;
+        }
 
         if (includes(consideredExpected, e)) {
             return true;
         }
         if (includes(consideredFailure, e)) {
             return false;
         }
+
         return true;
     }
 

implementation/core/src/main/java/io/smallrye/faulttolerance/core/util/SetOfThrowables.java

@@ -36,6 +36,18 @@ private SetOfThrowables(Set<Class<? extends Throwable>> classes) {
         this.classes = classes;
     }
 
+    boolean isEmpty() {
+        return classes.isEmpty();
+    }
+
+    boolean isAll() {
+        if (classes.size() == 1) {
+            Class<? extends Throwable> clazz = classes.iterator().next();
+            return clazz == Throwable.class || clazz == Exception.class;
+        }
+        return false;
+    }
+
     /**
      * @param searchedFor a class to check
      * @return whether {@code searchedFor} is a subtype of (at least) one of the types in this set.

implementation/fault-tolerance/src/main/java/io/smallrye/faulttolerance/FaultToleranceInterceptor.java

@@ -625,7 +625,7 @@ private ExceptionDecision createExceptionDecision(Class<? extends Throwable>[] c
     }
 
     private SetOfThrowables createSetOfThrowables(Class<? extends Throwable>[] throwableClasses) {
-        if (throwableClasses == null) {
+        if (throwableClasses == null || throwableClasses.length == 0) {
             return SetOfThrowables.EMPTY;
         }
         return SetOfThrowables.create(Arrays.asList(throwableClasses));

testsuite/basic/src/test/java/io/smallrye/faulttolerance/fallback/causechain/FallbackWithApplyOn.java

@@ -0,0 +1,18 @@
+package io.smallrye.faulttolerance.fallback.causechain;
+
+import java.io.IOException;
+
+import jakarta.enterprise.context.ApplicationScoped;
+
+import org.eclipse.microprofile.faulttolerance.Fallback;
+
+@ApplicationScoped
+public class FallbackWithApplyOn {
+    @Fallback(fallbackMethod = "fallback", applyOn = IOException.class)
+    public void hello(Exception e) throws Exception {
+        throw e;
+    }
+
+    public void fallback(Exception ignored) {
+    }
+}

testsuite/basic/src/test/java/io/smallrye/faulttolerance/fallback/causechain/MyService.java → testsuite/basic/src/test/java/io/smallrye/faulttolerance/fallback/causechain/FallbackWithBothSkipOnAndApplyOn.java

@@ -7,7 +7,7 @@
 import org.eclipse.microprofile.faulttolerance.Fallback;
 
 @ApplicationScoped
-public class MyService {
+public class FallbackWithBothSkipOnAndApplyOn {
     @Fallback(fallbackMethod = "fallback", skipOn = ExpectedOutcomeException.class, applyOn = IOException.class)
     public void hello(Exception e) throws Exception {
         throw e;