diff --git a/README.md b/README.md index 170d5db2..fa0978c0 100644 --- a/README.md +++ b/README.md @@ -597,6 +597,24 @@ When evaluating at runtime, a root object containing the method arguments is pas **Note:** The arguments are not available until the method has been called at least once; they will be null initially, which means, for example, you can't set the initial `maxAttempts` using an argument value, you can, however, change the `maxAttempts` after the first failure and before any retries are performed. Also, the arguments are only available when using stateless retry (which includes the `@CircuitBreaker`). +Version 2.0 adds more flexibility to exception classification. + +```java +@Retryable(retryFor = RuntimeException.class, noRetryFor = IllegalStateException.class, notRecoverable = { + IllegalArgumentException.class, IllegalStateException.class }) +public void service() { + ... +} + +@Recover +public void recover(Throwable cause) { + ... +} +``` + +`retryFor` and `noRetryFor` are replacements of `include` and `exclude` properties, which are now deprecated. +The new `notRecoverable` property allows the recovery method(s) to be skipped, even if one matches the exception type; the exception is thrown to the caller either after retries are exhausted, or immediately, if the exception is not retryable. + ##### Examples ```java diff --git a/src/main/java/org/springframework/retry/RetryContext.java b/src/main/java/org/springframework/retry/RetryContext.java index 9293ade0..cec6ac55 100644 --- a/src/main/java/org/springframework/retry/RetryContext.java +++ b/src/main/java/org/springframework/retry/RetryContext.java @@ -1,5 +1,5 @@ /* - * Copyright 2006-2007 the original author or authors. + * Copyright 2006-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -55,6 +55,12 @@ public interface RetryContext extends AttributeAccessor { */ String EXHAUSTED = "context.exhausted"; + /** + * Retry context attribute that is non-null (and true) if the exception is not + * recoverable. + */ + String NO_RECOVERY = "context.no-recovery"; + /** * Signal to the framework that no more attempts should be made to try or retry the * current {@link RetryCallback}. diff --git a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java index 0b80f2b2..dffa8b05 100644 --- a/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java +++ b/src/main/java/org/springframework/retry/annotation/AnnotationAwareRetryOperationsInterceptor.java @@ -344,11 +344,11 @@ private RetryPolicy getRetryPolicy(Annotation retryable, boolean stateless) { boolean hasExpression = StringUtils.hasText(exceptionExpression); if (includes.length == 0) { @SuppressWarnings("unchecked") - Class[] value = (Class[]) attrs.get("include"); + Class[] value = (Class[]) attrs.get("retryFor"); includes = value; } @SuppressWarnings("unchecked") - Class[] excludes = (Class[]) attrs.get("exclude"); + Class[] excludes = (Class[]) attrs.get("noRetryFor"); Integer maxAttempts = (Integer) attrs.get("maxAttempts"); String maxAttemptsExpression = (String) attrs.get("maxAttemptsExpression"); Expression parsedExpression = null; @@ -360,8 +360,9 @@ private RetryPolicy getRetryPolicy(Annotation retryable, boolean stateless) { } } final Expression expression = parsedExpression; + SimpleRetryPolicy simple = null; if (includes.length == 0 && excludes.length == 0) { - SimpleRetryPolicy simple = hasExpression + simple = hasExpression ? new ExpressionRetryPolicy(resolve(exceptionExpression)).withBeanFactory(this.beanFactory) : new SimpleRetryPolicy(); if (expression != null) { @@ -370,7 +371,6 @@ private RetryPolicy getRetryPolicy(Annotation retryable, boolean stateless) { else { simple.setMaxAttempts(maxAttempts); } - return simple; } Map, Boolean> policyMap = new HashMap<>(); for (Class type : includes) { @@ -380,17 +380,24 @@ private RetryPolicy getRetryPolicy(Annotation retryable, boolean stateless) { policyMap.put(type, false); } boolean retryNotExcluded = includes.length == 0; - if (hasExpression) { - return new ExpressionRetryPolicy(maxAttempts, policyMap, true, exceptionExpression, retryNotExcluded) - .withBeanFactory(this.beanFactory); - } - else { - SimpleRetryPolicy policy = new SimpleRetryPolicy(maxAttempts, policyMap, true, retryNotExcluded); - if (expression != null) { - policy.setMaxAttempts(() -> evaluate(expression, Integer.class, stateless)); + if (simple == null) { + if (hasExpression) { + simple = new ExpressionRetryPolicy(maxAttempts, policyMap, true, resolve(exceptionExpression), + retryNotExcluded).withBeanFactory(this.beanFactory); + } + else { + simple = new SimpleRetryPolicy(maxAttempts, policyMap, true, retryNotExcluded); + if (expression != null) { + simple.setMaxAttempts(() -> evaluate(expression, Integer.class, stateless)); + } } - return policy; } + @SuppressWarnings("unchecked") + Class[] noRecovery = (Class[]) attrs.get("notRecoverable"); + if (noRecovery != null && noRecovery.length > 0) { + simple.setNotRecoverable(noRecovery); + } + return simple; } private BackOffPolicy getBackoffPolicy(Backoff backoff, boolean stateless) { diff --git a/src/main/java/org/springframework/retry/annotation/CircuitBreaker.java b/src/main/java/org/springframework/retry/annotation/CircuitBreaker.java index 0df6a5a8..caa7bd63 100644 --- a/src/main/java/org/springframework/retry/annotation/CircuitBreaker.java +++ b/src/main/java/org/springframework/retry/annotation/CircuitBreaker.java @@ -22,6 +22,8 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import org.springframework.core.annotation.AliasFor; + /** * Annotation for a method invocation that is retryable. * @@ -48,17 +50,51 @@ * Exception types that are retryable. Defaults to empty (and if excludes is also * empty all exceptions are retried). * @return exception types to retry + * @deprecated in favor of {@link #retryFor()}. */ + @AliasFor("retryFor") + @Deprecated Class[] include() default {}; + /** + * Exception types that are retryable. Defaults to empty (and, if noRetryFor is also + * empty, all exceptions are retried). + * @return exception types to retry + * @since 2.0 + */ + @AliasFor("include") + Class[] retryFor() default {}; + /** * Exception types that are not retryable. Defaults to empty (and if includes is also * empty all exceptions are retried). If includes is empty but excludes is not, all * not excluded exceptions are retried * @return exception types not to retry + * @deprecated in favor of {@link #noRetryFor()}. */ + @Deprecated + @AliasFor("noRetryFor") Class[] exclude() default {}; + /** + * Exception types that are not retryable. Defaults to empty (and, if retryFor is also + * empty, all exceptions are retried). If retryFor is empty but excludes is not, all + * other exceptions are retried + * @return exception types not to retry + * @since 2.0 + */ + @AliasFor("exclude") + Class[] noRetryFor() default {}; + + /** + * Exception types that are not recoverable; these exceptions are thrown to the caller + * without calling any recoverer (immediately if also in {@link #noRetryFor()}). + * Defaults to empty. + * @return exception types not to retry + * @since 2.0 + */ + Class[] notRecoverable() default {}; + /** * @return the maximum number of attempts (including the first failure), defaults to 3 */ diff --git a/src/main/java/org/springframework/retry/annotation/Retryable.java b/src/main/java/org/springframework/retry/annotation/Retryable.java index 23c670ec..ae9af1f2 100644 --- a/src/main/java/org/springframework/retry/annotation/Retryable.java +++ b/src/main/java/org/springframework/retry/annotation/Retryable.java @@ -22,6 +22,8 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import org.springframework.core.annotation.AliasFor; + /** * Annotation for a method invocation that is retryable. * @@ -52,27 +54,63 @@ String interceptor() default ""; /** - * Exception types that are retryable. Synonym for includes(). Defaults to empty (and + * Exception types that are retryable. Synonym for include(). Defaults to empty (and * if excludes is also empty all exceptions are retried). * @return exception types to retry + * @deprecated in favor of {@link #retryFor()} */ + @Deprecated Class[] value() default {}; /** - * Exception types that are retryable. Defaults to empty (and if excludes is also - * empty all exceptions are retried). + * Exception types that are retryable. Defaults to empty (and, if exclude is also + * empty, all exceptions are retried). * @return exception types to retry + * @deprecated in favor of {@link #retryFor()}. */ + @AliasFor("retryFor") + @Deprecated Class[] include() default {}; /** - * Exception types that are not retryable. Defaults to empty (and if includes is also + * Exception types that are retryable. Defaults to empty (and, if noRetryFor is also + * empty, all exceptions are retried). + * @return exception types to retry + * @since 2.0 + */ + @AliasFor("include") + Class[] retryFor() default {}; + + /** + * Exception types that are not retryable. Defaults to empty (and if include is also * empty all exceptions are retried). If includes is empty but excludes is not, all * not excluded exceptions are retried * @return exception types not to retry + * @deprecated in favor of {@link #noRetryFor()}. */ + @Deprecated + @AliasFor("noRetryFor") Class[] exclude() default {}; + /** + * Exception types that are not retryable. Defaults to empty (and, if retryFor is also + * empty, all exceptions are retried). If retryFor is empty but excludes is not, all + * other exceptions are retried + * @return exception types not to retry + * @since 2.0 + */ + @AliasFor("exclude") + Class[] noRetryFor() default {}; + + /** + * Exception types that are not recoverable; these exceptions are thrown to the caller + * without calling any recoverer (immediately if also in {@link #noRetryFor()}). + * Defaults to empty. + * @return exception types not to retry + * @since 2.0 + */ + Class[] notRecoverable() default {}; + /** * A unique label for statistics reporting. If not provided the caller may choose to * ignore it, or provide a default. diff --git a/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java b/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java index 47a2c101..162c1543 100644 --- a/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java +++ b/src/main/java/org/springframework/retry/policy/SimpleRetryPolicy.java @@ -16,6 +16,8 @@ package org.springframework.retry.policy; +import java.util.Collections; +import java.util.HashMap; import java.util.Map; import java.util.function.Supplier; @@ -70,6 +72,9 @@ public class SimpleRetryPolicy implements RetryPolicy { private BinaryExceptionClassifier retryableClassifier = new BinaryExceptionClassifier(false); + private BinaryExceptionClassifier recoverableClassifier = new BinaryExceptionClassifier(Collections.emptyMap(), + true, true); + /** * Create a {@link SimpleRetryPolicy} with the default number of retry attempts, * retrying all exceptions. @@ -153,6 +158,20 @@ public void setMaxAttempts(int maxAttempts) { this.maxAttempts = maxAttempts; } + /** + * Configure throwables that should not be passed to a recoverer (if present) but + * thrown immediately. + * @param noRecovery the throwables. + * @since 3.0 + */ + public void setNotRecoverable(Class... noRecovery) { + Map, Boolean> map = new HashMap<>(); + for (Class clazz : noRecovery) { + map.put(clazz, false); + } + this.recoverableClassifier = new BinaryExceptionClassifier(map, true, true); + } + /** * Set a supplier for the number of attempts before retries are exhausted. Includes * the initial attempt before the retries begin so, generally, will be {@code >= 1}. @@ -190,7 +209,14 @@ public int getMaxAttempts() { @Override public boolean canRetry(RetryContext context) { Throwable t = context.getLastThrowable(); - return (t == null || retryForException(t)) && context.getRetryCount() < getMaxAttempts(); + boolean can = (t == null || retryForException(t)) && context.getRetryCount() < getMaxAttempts(); + if (!can && !this.recoverableClassifier.classify(t)) { + context.setAttribute(RetryContext.NO_RECOVERY, true); + } + else { + context.removeAttribute(RetryContext.NO_RECOVERY); + } + return can; } /** diff --git a/src/main/java/org/springframework/retry/support/RetryTemplate.java b/src/main/java/org/springframework/retry/support/RetryTemplate.java index 6c183c2d..abc1d90d 100644 --- a/src/main/java/org/springframework/retry/support/RetryTemplate.java +++ b/src/main/java/org/springframework/retry/support/RetryTemplate.java @@ -537,20 +537,27 @@ protected T handleRetryExhausted(RecoveryCallback recoveryCallback, Retry if (state != null && !context.hasAttribute(GLOBAL_STATE)) { this.retryContextCache.remove(state.getKey()); } + boolean doRecover = !Boolean.TRUE.equals(context.getAttribute(RetryContext.NO_RECOVERY)); if (recoveryCallback != null) { - T recovered = recoveryCallback.recover(context); - context.setAttribute(RetryContext.RECOVERED, true); - return recovered; + if (doRecover) { + T recovered = recoveryCallback.recover(context); + context.setAttribute(RetryContext.RECOVERED, true); + return recovered; + } + else { + logger.debug("Retry exhausted and recovery disabled for this throwable"); + } } if (state != null) { this.logger.debug("Retry exhausted after last attempt with no recovery path."); - rethrow(context, "Retry exhausted after last attempt with no recovery path"); + rethrow(context, "Retry exhausted after last attempt with no recovery path", + this.throwLastExceptionOnExhausted || !doRecover); } throw wrapIfNecessary(context.getLastThrowable()); } - protected void rethrow(RetryContext context, String message) throws E { - if (this.throwLastExceptionOnExhausted) { + protected void rethrow(RetryContext context, String message, boolean wrap) throws E { + if (wrap) { @SuppressWarnings("unchecked") E rethrow = (E) context.getLastThrowable(); throw rethrow; diff --git a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java index bae4aceb..0d234140 100644 --- a/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java +++ b/src/test/java/org/springframework/retry/annotation/EnableRetryTests.java @@ -48,7 +48,6 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.assertj.core.api.Assertions.setMaxStackTraceElementsDisplayed; -import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -121,7 +120,13 @@ public void recovery() { RecoverableService service = context.getBean(RecoverableService.class); service.service(); assertThat(service.getCount()).isEqualTo(3); - assertNotNull(service.getCause()); + assertThat(service.getCause()).isExactlyInstanceOf(RuntimeException.class); + assertThatIllegalArgumentException().isThrownBy(() -> service.service()); + assertThat(service.getCount()).isEqualTo(6); + assertThat(service.getCause()).isExactlyInstanceOf(RuntimeException.class); + assertThatIllegalStateException().isThrownBy(() -> service.service()); + assertThat(service.getCount()).isEqualTo(7); + assertThat(service.getCause()).isExactlyInstanceOf(RuntimeException.class); context.close(); } @@ -582,10 +587,18 @@ protected static class RecoverableService { boolean otherAdviceCalled; - @Retryable(RuntimeException.class) + @Retryable(retryFor = RuntimeException.class, noRetryFor = IllegalStateException.class, + notRecoverable = { IllegalArgumentException.class, IllegalStateException.class }) public void service() { - this.count++; - throw new RuntimeException("Planned"); + if (this.count++ >= 3 && count < 7) { + throw new IllegalArgumentException("Planned"); + } + else if (count > 6) { + throw new IllegalStateException("Planned"); + } + else { + throw new RuntimeException("Planned"); + } } @Recover @@ -632,7 +645,7 @@ protected static class ExcludesService { private int count = 0; - @Retryable(include = RuntimeException.class, exclude = IllegalStateException.class) + @Retryable(retryFor = RuntimeException.class, noRetryFor = IllegalStateException.class) public void service() { if (this.count++ < 2) { throw new IllegalStateException("Planned"); @@ -651,7 +664,7 @@ protected static class ExcludesOnlyService { private RuntimeException exceptionToThrow; - @Retryable(exclude = IllegalStateException.class) + @Retryable(noRetryFor = IllegalStateException.class) public void service() { if (this.count++ < 2) { throw this.exceptionToThrow; diff --git a/src/test/java/org/springframework/retry/support/RetryTemplateTests.java b/src/test/java/org/springframework/retry/support/RetryTemplateTests.java index 06a0a75b..57cf9423 100644 --- a/src/test/java/org/springframework/retry/support/RetryTemplateTests.java +++ b/src/test/java/org/springframework/retry/support/RetryTemplateTests.java @@ -261,7 +261,7 @@ public void registerThrowable(RetryContext context, Throwable throwable) { }); assertThatExceptionOfType(TerminatedRetryException.class).isThrownBy(() -> retryTemplate.execute(context -> { throw new RuntimeException("Realllly bad!"); - })).withCauseInstanceOf(RuntimeException.class).withMessageContaining("Planned"); + })).withCauseInstanceOf(RuntimeException.class).withStackTraceContaining("Planned"); } @Test diff --git a/src/test/java/org/springframework/retry/support/StatefulRecoveryRetryTests.java b/src/test/java/org/springframework/retry/support/StatefulRecoveryRetryTests.java index 4458d44f..23fada5f 100644 --- a/src/test/java/org/springframework/retry/support/StatefulRecoveryRetryTests.java +++ b/src/test/java/org/springframework/retry/support/StatefulRecoveryRetryTests.java @@ -170,7 +170,7 @@ public void testKeyGeneratorNotConsistentAfterFailure() throws Throwable { // inconsistent has codes relies on the cache having been used for this // item already... assertThatExceptionOfType(RetryException.class).isThrownBy(() -> this.retryTemplate.execute(callback, state)) - .withMessageContaining("inconsistent"); + .withStackTraceContaining("inconsistent"); RetryContext context = this.retryTemplate.open(retryPolicy, state); // True after exhausted - the history is reset... @@ -195,7 +195,7 @@ public void testCacheCapacity() throws Throwable { assertThatExceptionOfType(RetryException.class) .isThrownBy(() -> this.retryTemplate.execute(callback, new DefaultRetryState("bar"))) - .withMessageContaining("capacity"); + .withStackTraceContaining("capacity"); } @Test