Merge pull request #1007 from Ladicek/retry-when

add @RetryWhen

api/src/main/java/io/smallrye/faulttolerance/api/AlwaysOnException.java

@@ -0,0 +1,10 @@
+package io.smallrye.faulttolerance.api;
+
+import java.util.function.Predicate;
+
+public final class AlwaysOnException implements Predicate<Throwable> {
+    @Override
+    public boolean test(Throwable ignored) {
+        return true;
+    }
+}

api/src/main/java/io/smallrye/faulttolerance/api/FaultTolerance.java

@@ -838,6 +838,24 @@ default RetryBuilder<T, R> abortOn(Class<? extends Throwable> value) {
                 return abortOn(Collections.singleton(Objects.requireNonNull(value)));
             }
 
+            /**
+             * Sets a predicate to determine when a result should be considered failure and retry
+             * should be attempted. All results that do not match this predicate are implicitly
+             * considered success.
+             *
+             * @param value the predicate, must not be {@code null}
+             * @return this retry builder
+             */
+            RetryBuilder<T, R> whenResult(Predicate<Object> value);
+
+            /**
+             * @deprecated use {@link #whenException(Predicate)}
+             */
+            @Deprecated(forRemoval = true)
+            default RetryBuilder<T, R> when(Predicate<Throwable> value) {
+                return whenException(value);
+            }
+
             /**
              * Sets a predicate to determine when an exception should be considered failure
              * and retry should be attempted. This is a more general variant of {@link #retryOn(Collection) retryOn}.
@@ -847,9 +865,9 @@ default RetryBuilder<T, R> abortOn(Class<? extends Throwable> value) {
              * If this method is called, {@code retryOn} and {@code abortOn} may not be called.
              *
              * @param value the predicate, must not be {@code null}
-             * @return this fallback builder
+             * @return this retry builder
              */
-            RetryBuilder<T, R> when(Predicate<Throwable> value);
+            RetryBuilder<T, R> whenException(Predicate<Throwable> value);
 
             /**
              * Configures retry to use an exponential backoff instead of the default constant backoff.

api/src/main/java/io/smallrye/faulttolerance/api/NeverOnResult.java

@@ -0,0 +1,10 @@
+package io.smallrye.faulttolerance.api;
+
+import java.util.function.Predicate;
+
+public final class NeverOnResult implements Predicate<Object> {
+    @Override
+    public boolean test(Object ignored) {
+        return false;
+    }
+}

api/src/main/java/io/smallrye/faulttolerance/api/RetryWhen.java

@@ -0,0 +1,53 @@
+package io.smallrye.faulttolerance.api;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Inherited;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import java.util.function.Predicate;
+
+import io.smallrye.common.annotation.Experimental;
+
+/**
+ * Modifies a {@code @Retry} annotation to retry when given predicate returns {@code true}.
+ * May only be present on elements that are also annotated {@code @Retry}. If this annotation
+ * is present and the {@code @RetryWhen.exception} member is set, the {@code @Retry.retryOn}
+ * and {@code @Retry.abortOn} members must not be set.
+ * <p>
+ * For each usage of the {@code @RetryWhen} annotation, all given {@link Predicate}s are
+ * instantiated once. The predicate classes must have a {@code public}, zero-parameter
+ * constructor. They must be thread-safe, ideally stateless.
+ *
+ * @see #exception()
+ * @see #result()
+ */
+@Inherited
+@Documented
+@Retention(RetentionPolicy.RUNTIME)
+@Target({ ElementType.METHOD, ElementType.TYPE })
+@Experimental("first attempt at providing predicate-based retries")
+public @interface RetryWhen {
+    /**
+     * Class of the predicate that will be used to determine whether the action should be retried
+     * if the action has returned a result.
+     * <p>
+     * Even if the guarded action is asynchronous, the predicate takes a produced result.
+     * The predicate is never passed a {@link java.util.concurrent.CompletionStage CompletionStage} or so.
+     *
+     * @return predicate class
+     */
+    Class<? extends Predicate<Object>> result() default NeverOnResult.class;
+
+    /**
+     * Class of the predicate that will be used to determine whether the action should be retried
+     * if the action has thrown an exception.
+     * <p>
+     * Even if the guarded action is asynchronous, the predicate takes a produced exception.
+     * The predicate is never passed a {@link java.util.concurrent.CompletionStage CompletionStage} or so.
+     *
+     * @return predicate class
+     */
+    Class<? extends Predicate<Throwable>> exception() default AlwaysOnException.class;
+}

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

@@ -95,7 +95,7 @@ This configuration takes precedence over `retryOn`.
 [[metrics]]
 == Metrics
 
-Rate limit exposes the following metrics:
+Retry exposes the following metrics:
 
 [cols="1,5"]
 |===
@@ -183,6 +183,59 @@ This is an advanced option.
 
 For more information about these backoff strategies, see the javadoc of the annotations.
 
+=== Predicate-Based `@Retry`
+
+include::partial$srye-feature.adoc[]
+
+By default, an operation is retried only if it fails and the exception is assignable to one of the classes defined in `@Retry.retryOn` (and not assignable to any of the classes defined in `@Retry.abortOn`).
+With the `@RetryWhen` annotation, it is possible to instead define a custom predicate for the exception, as well as a predicate for the result.
+
+The `@RetryWhen` annotation may be present on any program element (method or class) that also has the `@Retry` annotation.
+For example:
+
+[source,java]
+----
+package com.example;
+
+@ApplicationScoped
+public class MyService {
+    @Retry
+    @RetryWhen(
+        result = IsNull.class, // <1>
+        exception = IsRuntimeException.class // <2>
+    )
+    public String hello() {
+        ...
+    }
+
+    public static final class IsNull implements Predicate<Object> {
+        @Override
+        public boolean test(Object o) {
+            return o == null;
+        }
+    }
+
+    public static final class IsRuntimeException implements Predicate<Throwable> {
+        @Override
+        public boolean test(Throwable throwable) {
+            return throwable instanceof RuntimeException;
+        }
+    }
+}
+----
+
+<1> When the method returns `null`, it will be retried.
+<2> When the method throws a `RuntimeException`, it will be retried.
+
+All other results are considered expected and are not retried.
+
+It is an error to specify `@RetryWhen.exception` if `@Retry.retryOn` / `@Retry.abortOn` is specified.
+Specifying `@RetryWhen.result` together with `@Retry.retryOn` / `@Retry.abortOn` is possible, although perhaps not the best idea.
+
+It is an error to add a `@RetryWhen` annotation to a program element that doesn't have `@Retry` (e.g. add `@Retry` on a class and `@RetryWhen` on a method).
+
+For more information about `@RetryWhen`, see the javadoc of the annotation.
+
 [[inspecting-exception-cause-chains]]
 === Inspecting Exception Cause Chains
 

implementation/autoconfig/core/src/main/java/io/smallrye/faulttolerance/autoconfig/FaultToleranceMethod.java

@@ -19,6 +19,7 @@
 import io.smallrye.faulttolerance.api.ExponentialBackoff;
 import io.smallrye.faulttolerance.api.FibonacciBackoff;
 import io.smallrye.faulttolerance.api.RateLimit;
+import io.smallrye.faulttolerance.api.RetryWhen;
 
 /**
  * Created in the CDI extension to capture effective annotations for each
@@ -57,6 +58,7 @@ public class FaultToleranceMethod {
     public CustomBackoff customBackoff;
     public ExponentialBackoff exponentialBackoff;
     public FibonacciBackoff fibonacciBackoff;
+    public RetryWhen retryWhen;
 
     // types of annotations that were declared directly on the method;
     // other annotations, if present, were declared on the type
@@ -67,7 +69,7 @@ public boolean isLegitimate() {
             return false;
         }
 
-        // certain SmallRye annotations (@CircuitBreakerName, @[Non]Blocking, @*Backoff)
+        // certain SmallRye annotations (@CircuitBreakerName, @[Non]Blocking, @*Backoff, @RetryWhen)
         // do _not_ trigger the fault tolerance interceptor alone, only in combination
         // with other fault tolerance annotations
         return applyFaultTolerance != null

implementation/core/src/main/java/io/smallrye/faulttolerance/core/apiimpl/FaultToleranceImpl.java

@@ -60,6 +60,8 @@
 import io.smallrye.faulttolerance.core.util.ExceptionDecision;
 import io.smallrye.faulttolerance.core.util.Preconditions;
 import io.smallrye.faulttolerance.core.util.PredicateBasedExceptionDecision;
+import io.smallrye.faulttolerance.core.util.PredicateBasedResultDecision;
+import io.smallrye.faulttolerance.core.util.ResultDecision;
 import io.smallrye.faulttolerance.core.util.SetBasedExceptionDecision;
 import io.smallrye.faulttolerance.core.util.SetOfThrowables;
 
@@ -306,7 +308,9 @@ private FaultToleranceStrategy<T> buildSyncStrategy(BuilderLazyDependencies lazy
                 Supplier<BackOff> backoff = prepareRetryBackoff(retryBuilder);
 
                 result = new Retry<>(result, description,
-                        createExceptionDecision(retryBuilder.abortOn, retryBuilder.retryOn, retryBuilder.whenPredicate),
+                        createResultDecision(retryBuilder.whenResultPredicate),
+                        createExceptionDecision(retryBuilder.abortOn, retryBuilder.retryOn,
+                                retryBuilder.whenExceptionPredicate),
                         retryBuilder.maxRetries, retryBuilder.maxDurationInMillis, () -> new ThreadSleepDelay(backoff.get()),
                         SystemStopwatch.INSTANCE);
             }
@@ -377,7 +381,9 @@ private <V> FaultToleranceStrategy<CompletionStage<V>> buildAsyncStrategy(Builde
                 Supplier<BackOff> backoff = prepareRetryBackoff(retryBuilder);
 
                 result = new CompletionStageRetry<>(result, description,
-                        createExceptionDecision(retryBuilder.abortOn, retryBuilder.retryOn, retryBuilder.whenPredicate),
+                        createResultDecision(retryBuilder.whenResultPredicate),
+                        createExceptionDecision(retryBuilder.abortOn, retryBuilder.retryOn,
+                                retryBuilder.whenExceptionPredicate),
                         retryBuilder.maxRetries, retryBuilder.maxDurationInMillis,
                         () -> new TimerDelay(backoff.get(), lazyDependencies.timer()),
                         SystemStopwatch.INSTANCE);
@@ -418,13 +424,23 @@ private MeteredOperation buildMeteredOperation() {
                     retryBuilder != null, timeoutBuilder != null);
         }
 
+        private static ResultDecision createResultDecision(Predicate<Object> whenResultPredicate) {
+            if (whenResultPredicate != null) {
+                // the builder API accepts a predicate that returns `true` when a result is considered failure,
+                // but `[CompletionStage]Retry` accepts a predicate that returns `true` when a result is
+                // considered success -- hence the negation
+                return new PredicateBasedResultDecision(whenResultPredicate.negate());
+            }
+            return ResultDecision.ALWAYS_EXPECTED;
+        }
+
         private static ExceptionDecision createExceptionDecision(Collection<Class<? extends Throwable>> consideredExpected,
-                Collection<Class<? extends Throwable>> consideredFailure, Predicate<Throwable> whenPredicate) {
-            if (whenPredicate != null) {
+                Collection<Class<? extends Throwable>> consideredFailure, Predicate<Throwable> whenExceptionPredicate) {
+            if (whenExceptionPredicate != null) {
                 // the builder API accepts a predicate that returns `true` when an exception is considered failure,
                 // but `PredicateBasedExceptionDecision` accepts a predicate that returns `true` when an exception
                 // is considered success -- hence the negation
-                return new PredicateBasedExceptionDecision(whenPredicate.negate());
+                return new PredicateBasedExceptionDecision(whenExceptionPredicate.negate());
             }
             return new SetBasedExceptionDecision(createSetOfThrowables(consideredFailure),
                     createSetOfThrowables(consideredExpected), true);
@@ -756,7 +772,8 @@ static class RetryBuilderImpl<T, R> implements RetryBuilder<T, R> {
             private Collection<Class<? extends Throwable>> retryOn = Collections.singleton(Exception.class);
             private Collection<Class<? extends Throwable>> abortOn = Collections.emptySet();
             private boolean setBasedExceptionDecisionDefined = false;
-            private Predicate<Throwable> whenPredicate;
+            private Predicate<Throwable> whenExceptionPredicate;
+            private Predicate<Object> whenResultPredicate;
 
             private ExponentialBackoffBuilderImpl<T, R> exponentialBackoffBuilder;
             private FibonacciBackoffBuilderImpl<T, R> fibonacciBackoffBuilder;
@@ -818,8 +835,14 @@ public RetryBuilder<T, R> abortOn(Collection<Class<? extends Throwable>> value)
             }
 
             @Override
-            public RetryBuilder<T, R> when(Predicate<Throwable> value) {
-                this.whenPredicate = Preconditions.checkNotNull(value, "Exception predicate must be set");
+            public RetryBuilder<T, R> whenResult(Predicate<Object> value) {
+                this.whenResultPredicate = Preconditions.checkNotNull(value, "Result predicate must be set");
+                return this;
+            }
+
+            @Override
+            public RetryBuilder<T, R> whenException(Predicate<Throwable> value) {
+                this.whenExceptionPredicate = Preconditions.checkNotNull(value, "Exception predicate must be set");
                 return this;
             }
 
@@ -858,8 +881,8 @@ public RetryBuilder<T, R> onFailure(Runnable callback) {
 
             @Override
             public Builder<T, R> done() {
-                if (whenPredicate != null && setBasedExceptionDecisionDefined) {
-                    throw new IllegalStateException("The when() method may not be combined with retryOn() / abortOn()");
+                if (whenExceptionPredicate != null && setBasedExceptionDecisionDefined) {
+                    throw new IllegalStateException("The whenException() method may not be combined with retryOn()/abortOn()");
                 }
 
                 int backoffStrategies = 0;

implementation/core/src/main/java/io/smallrye/faulttolerance/core/retry/CompletionStageRetry.java

@@ -17,15 +17,17 @@
 import io.smallrye.faulttolerance.core.stopwatch.RunningStopwatch;
 import io.smallrye.faulttolerance.core.stopwatch.Stopwatch;
 import io.smallrye.faulttolerance.core.util.ExceptionDecision;
+import io.smallrye.faulttolerance.core.util.ResultDecision;
 
 public class CompletionStageRetry<V> extends Retry<CompletionStage<V>> {
     private final Supplier<AsyncDelay> delayBetweenRetries;
 
     public CompletionStageRetry(FaultToleranceStrategy<CompletionStage<V>> delegate, String description,
-            ExceptionDecision exceptionDecision, long maxRetries, long maxTotalDurationInMillis,
-            Supplier<AsyncDelay> delayBetweenRetries, Stopwatch stopwatch) {
+            ResultDecision resultDecision, ExceptionDecision exceptionDecision, long maxRetries,
+            long maxTotalDurationInMillis, Supplier<AsyncDelay> delayBetweenRetries, Stopwatch stopwatch) {
         // the SyncDelay.NONE is ignored here, we have our own AsyncDelay
-        super(delegate, description, exceptionDecision, maxRetries, maxTotalDurationInMillis, SyncDelay.NONE, stopwatch);
+        super(delegate, description, resultDecision, exceptionDecision, maxRetries, maxTotalDurationInMillis,
+                SyncDelay.NONE, stopwatch);
         this.delayBetweenRetries = checkNotNull(delayBetweenRetries, "Delay must be set");
     }
 
@@ -87,10 +89,14 @@ private CompletionStage<V> afterDelay(InvocationContext<CompletionStage<V>> ctx,
 
             delegate.apply(ctx).whenComplete((value, exception) -> {
                 if (exception == null) {
-                    ctx.fireEvent(RetryEvents.Finished.VALUE_RETURNED);
-                    result.complete(value);
+                    if (shouldAbortRetryingOnResult(value)) {
+                        ctx.fireEvent(RetryEvents.Finished.VALUE_RETURNED);
+                        result.complete(value);
+                    } else {
+                        propagateCompletion(doRetry(ctx, attempt + 1, delay, stopwatch, exception), result);
+                    }
                 } else {
-                    if (shouldAbortRetrying(exception)) {
+                    if (shouldAbortRetryingOnException(exception)) {
                         ctx.fireEvent(RetryEvents.Finished.EXCEPTION_NOT_RETRYABLE);
                         result.completeExceptionally(exception);
                     } else {
@@ -101,7 +107,7 @@ private CompletionStage<V> afterDelay(InvocationContext<CompletionStage<V>> ctx,
 
             return result;
         } catch (Throwable e) {
-            if (shouldAbortRetrying(e)) {
+            if (shouldAbortRetryingOnException(e)) {
                 ctx.fireEvent(RetryEvents.Finished.EXCEPTION_NOT_RETRYABLE);
                 return failedFuture(e);
             } else {