From 97ae5fde7ca43ae5a799c3853ff352d64b39c595 Mon Sep 17 00:00:00 2001 From: Sam Brannen <104798+sbrannen@users.noreply.github.com> Date: Thu, 9 Oct 2025 16:53:23 +0200 Subject: [PATCH] =?UTF-8?q?Match=20against=20exception=20causes=20in=20@?= =?UTF-8?q?=E2=81=A0Retryable=20and=20RetryPolicy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior to this commit, our @⁠Retryable support as well as a RetryPolicy created by the RetryPolicy.Builder only matched against top-level exceptions when filtering included/excluded exceptions thrown by a @⁠Retryable method or Retryable operation. With this commit, we now match against not only top-level exceptions but also nested causes within those top-level exceptions. This is achieved via the new ExceptionTypeFilter.match(Throwable, boolean) support. See gh-35592 Closes gh-35583 --- .../modules/ROOT/pages/core/resilience.adoc | 11 ++-- .../resilience/annotation/Retryable.java | 7 +++ .../resilience/retry/MethodRetrySpec.java | 2 +- .../ReactiveRetryInterceptorTests.java | 50 ++++++++++++----- ...jectMalformedInputException3Predicate.java | 31 ++++++++++ .../resilience/RetryInterceptorTests.java | 56 +++++++++++++------ .../core/retry/DefaultRetryPolicy.java | 2 +- .../core/retry/RetryPolicy.java | 12 ++++ .../core/retry/RetryTemplateTests.java | 13 +++-- 9 files changed, 140 insertions(+), 44 deletions(-) create mode 100644 spring-context/src/test/java/org/springframework/resilience/RejectMalformedInputException3Predicate.java diff --git a/framework-docs/modules/ROOT/pages/core/resilience.adoc b/framework-docs/modules/ROOT/pages/core/resilience.adoc index c937c6983e2..4004064800f 100644 --- a/framework-docs/modules/ROOT/pages/core/resilience.adoc +++ b/framework-docs/modules/ROOT/pages/core/resilience.adoc @@ -26,8 +26,10 @@ public void sendNotification() { By default, the method invocation will be retried for any exception thrown: with at most 3 retry attempts after an initial failure, and a delay of 1 second between attempts. -This can be specifically adapted for every method if necessary – for example, by narrowing -the exceptions to retry: +This can be specifically adapted for every method if necessary — for example, by narrowing +the exceptions to retry via the `includes` and `excludes` attributes. The supplied +exception types will be matched against an exception thrown by a failed invocation as well +as nested causes. [source,java,indent=0,subs="verbatim,quotes"] ---- @@ -182,7 +184,8 @@ If you only need to customize the number of retry attempts, you can use the <1> Explicitly uses `RetryPolicy.withMaxAttempts(5)`. If you need to narrow the types of exceptions to retry, that can be achieved via the -`includes()` and `excludes()` builder methods. +`includes()` and `excludes()` builder methods. The supplied exception types will be +matched against an exception thrown by a failed operation as well as nested causes. [source,java,indent=0,subs="verbatim,quotes"] ---- @@ -204,7 +207,7 @@ If you need to narrow the types of exceptions to retry, that can be achieved via For advanced use cases, you can specify a custom `Predicate` via the `predicate()` method in the `RetryPolicy.Builder`, and the predicate will be used to determine whether to retry a failed operation based on a given `Throwable` – for example, -by checking the cause or the message of the `Throwable`. +by checking the message of the `Throwable`. Custom predicates can be combined with `includes` and `excludes`; however, custom predicates will always be applied after `includes` and `excludes` have been applied. diff --git a/spring-context/src/main/java/org/springframework/resilience/annotation/Retryable.java b/spring-context/src/main/java/org/springframework/resilience/annotation/Retryable.java index b44dcdf6124..8c174538de1 100644 --- a/spring-context/src/main/java/org/springframework/resilience/annotation/Retryable.java +++ b/spring-context/src/main/java/org/springframework/resilience/annotation/Retryable.java @@ -40,6 +40,7 @@ import org.springframework.resilience.retry.MethodRetryPredicate; * project but redesigned as a minimal core retry feature in the Spring Framework. * * @author Juergen Hoeller + * @author Sam Brannen * @since 7.0 * @see EnableResilientMethods * @see RetryAnnotationBeanPostProcessor @@ -64,6 +65,9 @@ public @interface Retryable { /** * Applicable exception types to attempt a retry for. This attribute * allows for the convenient specification of assignable exception types. + *

The supplied exception types will be matched against an exception + * thrown by a failed invocation as well as nested + * {@linkplain Throwable#getCause() causes}. *

This can optionally be combined with {@link #excludes() excludes} or * a custom {@link #predicate() predicate}. *

The default is empty, leading to a retry attempt for any exception. @@ -76,6 +80,9 @@ public @interface Retryable { /** * Non-applicable exception types to avoid a retry for. This attribute * allows for the convenient specification of assignable exception types. + *

The supplied exception types will be matched against an exception + * thrown by a failed invocation as well as nested + * {@linkplain Throwable#getCause() causes}. *

This can optionally be combined with {@link #includes() includes} or * a custom {@link #predicate() predicate}. *

The default is empty, leading to a retry attempt for any exception. diff --git a/spring-context/src/main/java/org/springframework/resilience/retry/MethodRetrySpec.java b/spring-context/src/main/java/org/springframework/resilience/retry/MethodRetrySpec.java index a453c85c4f6..b6d73192109 100644 --- a/spring-context/src/main/java/org/springframework/resilience/retry/MethodRetrySpec.java +++ b/spring-context/src/main/java/org/springframework/resilience/retry/MethodRetrySpec.java @@ -65,7 +65,7 @@ public record MethodRetrySpec( MethodRetryPredicate combinedPredicate() { ExceptionTypeFilter exceptionFilter = new ExceptionTypeFilter(this.includes, this.excludes); - return (method, throwable) -> exceptionFilter.match(throwable) && + return (method, throwable) -> exceptionFilter.match(throwable, true) && this.predicate.shouldRetry(method, throwable); } diff --git a/spring-context/src/test/java/org/springframework/resilience/ReactiveRetryInterceptorTests.java b/spring-context/src/test/java/org/springframework/resilience/ReactiveRetryInterceptorTests.java index ff81f8ca107..a289b79112b 100644 --- a/spring-context/src/test/java/org/springframework/resilience/ReactiveRetryInterceptorTests.java +++ b/spring-context/src/test/java/org/springframework/resilience/ReactiveRetryInterceptorTests.java @@ -17,7 +17,7 @@ package org.springframework.resilience; import java.io.IOException; -import java.lang.reflect.Method; +import java.nio.charset.MalformedInputException; import java.nio.file.AccessDeniedException; import java.nio.file.FileSystemException; import java.time.Duration; @@ -35,7 +35,6 @@ import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.resilience.annotation.RetryAnnotationBeanPostProcessor; import org.springframework.resilience.annotation.Retryable; -import org.springframework.resilience.retry.MethodRetryPredicate; import org.springframework.resilience.retry.MethodRetrySpec; import org.springframework.resilience.retry.SimpleRetryInterceptor; @@ -96,13 +95,16 @@ class ReactiveRetryInterceptorTests { // Exact includes match: IOException assertThatRuntimeException() .isThrownBy(() -> proxy.ioOperation().block()) - // Does NOT throw a RetryExhaustedException, because IOException3Predicate - // returns false once the exception's message is "3". + // Does NOT throw a RetryExhaustedException, because RejectMalformedInputException3Predicate + // rejects a retry if the last exception was a MalformedInputException with message "3". .satisfies(isReactiveException()) .havingCause() - .isInstanceOf(IOException.class) - .withMessage("3"); - // 1 initial attempt + 2 retries + .isInstanceOf(MalformedInputException.class) + .withMessageContaining("3"); + + // 3 = 1 initial invocation + 2 retry attempts + // Not 3 retry attempts, because RejectMalformedInputException3Predicate rejects + // a retry if the last exception was a MalformedInputException with message "3". assertThat(target.counter.get()).isEqualTo(3); } @@ -120,6 +122,22 @@ class ReactiveRetryInterceptorTests { assertThat(target.counter.get()).isEqualTo(4); } + @Test // gh-35583 + void withPostProcessorForClassWithCauseIncludesMatch() { + AnnotatedClassBean proxy = getProxiedAnnotatedClassBean(); + AnnotatedClassBean target = (AnnotatedClassBean) AopProxyUtils.getSingletonTarget(proxy); + + // Subtype includes match: FileSystemException + assertThatRuntimeException() + .isThrownBy(() -> proxy.fileSystemOperationWithNestedException().block()) + .satisfies(isRetryExhaustedException()) + .havingCause() + .isExactlyInstanceOf(RuntimeException.class) + .withCauseExactlyInstanceOf(FileSystemException.class); + // 1 initial attempt + 3 retries + assertThat(target.counter.get()).isEqualTo(4); + } + @Test void withPostProcessorForClassWithExcludesMatch() { AnnotatedClassBean proxy = getProxiedAnnotatedClassBean(); @@ -350,7 +368,7 @@ class ReactiveRetryInterceptorTests { @Retryable(delay = 10, jitter = 5, multiplier = 2.0, maxDelay = 40, includes = IOException.class, excludes = AccessDeniedException.class, - predicate = IOException3Predicate.class) + predicate = RejectMalformedInputException3Predicate.class) static class AnnotatedClassBean { AtomicInteger counter = new AtomicInteger(); @@ -358,6 +376,9 @@ class ReactiveRetryInterceptorTests { public Mono ioOperation() { return Mono.fromCallable(() -> { counter.incrementAndGet(); + if (counter.get() == 3) { + throw new MalformedInputException(counter.get()); + } throw new IOException(counter.toString()); }); } @@ -369,6 +390,13 @@ class ReactiveRetryInterceptorTests { }); } + public Mono fileSystemOperationWithNestedException() { + return Mono.fromCallable(() -> { + counter.incrementAndGet(); + throw new RuntimeException(new FileSystemException(counter.toString())); + }); + } + public Mono accessOperation() { return Mono.fromCallable(() -> { counter.incrementAndGet(); @@ -393,13 +421,7 @@ class ReactiveRetryInterceptorTests { } - private static class IOException3Predicate implements MethodRetryPredicate { - @Override - public boolean shouldRetry(Method method, Throwable throwable) { - return !(throwable.getClass() == IOException.class && "3".equals(throwable.getMessage())); - } - } // Bean classes for boundary testing diff --git a/spring-context/src/test/java/org/springframework/resilience/RejectMalformedInputException3Predicate.java b/spring-context/src/test/java/org/springframework/resilience/RejectMalformedInputException3Predicate.java new file mode 100644 index 00000000000..7a1b3498a44 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/resilience/RejectMalformedInputException3Predicate.java @@ -0,0 +1,31 @@ +/* + * Copyright 2002-present 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.resilience; + +import java.lang.reflect.Method; +import java.nio.charset.MalformedInputException; + +import org.springframework.resilience.retry.MethodRetryPredicate; + +class RejectMalformedInputException3Predicate implements MethodRetryPredicate { + + @Override + public boolean shouldRetry(Method method, Throwable throwable) { + return !(throwable.getClass() == MalformedInputException.class && throwable.getMessage().contains("3")); + } + +} diff --git a/spring-context/src/test/java/org/springframework/resilience/RetryInterceptorTests.java b/spring-context/src/test/java/org/springframework/resilience/RetryInterceptorTests.java index e637c297fa5..456687a7579 100644 --- a/spring-context/src/test/java/org/springframework/resilience/RetryInterceptorTests.java +++ b/spring-context/src/test/java/org/springframework/resilience/RetryInterceptorTests.java @@ -18,7 +18,7 @@ package org.springframework.resilience; import java.io.IOException; import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; +import java.nio.charset.MalformedInputException; import java.nio.file.AccessDeniedException; import java.time.Duration; import java.util.Properties; @@ -42,15 +42,16 @@ import org.springframework.resilience.annotation.ConcurrencyLimit; import org.springframework.resilience.annotation.EnableResilientMethods; import org.springframework.resilience.annotation.RetryAnnotationBeanPostProcessor; import org.springframework.resilience.annotation.Retryable; -import org.springframework.resilience.retry.MethodRetryPredicate; import org.springframework.resilience.retry.MethodRetrySpec; import org.springframework.resilience.retry.SimpleRetryInterceptor; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIOException; +import static org.assertj.core.api.Assertions.assertThatRuntimeException; /** * @author Juergen Hoeller + * @author Sam Brannen * @since 7.0 */ class RetryInterceptorTests { @@ -187,12 +188,22 @@ class RetryInterceptorTests { AnnotatedClassBean proxy = bf.getBean(AnnotatedClassBean.class); AnnotatedClassBean target = (AnnotatedClassBean) AopProxyUtils.getSingletonTarget(proxy); - assertThatIOException().isThrownBy(proxy::retryOperation).withMessage("3"); + // 3 = 1 initial invocation + 2 retry attempts + // Not 3 retry attempts, because RejectMalformedInputException3Predicate rejects + // a retry if the last exception was a MalformedInputException with message "3". + assertThatIOException().isThrownBy(proxy::retryOperation).withMessageContaining("3"); assertThat(target.counter).isEqualTo(3); + // 7 = 3 + 1 initial invocation + 3 retry attempts + assertThatRuntimeException() + .isThrownBy(proxy::retryOperationWithNestedException) + .havingCause() + .isExactlyInstanceOf(IOException.class) + .withMessage("7"); + assertThat(target.counter).isEqualTo(7); assertThatIOException().isThrownBy(proxy::otherOperation); - assertThat(target.counter).isEqualTo(4); + assertThat(target.counter).isEqualTo(8); assertThatIOException().isThrownBy(proxy::overrideOperation); - assertThat(target.counter).isEqualTo(6); + assertThat(target.counter).isEqualTo(10); } @Test @@ -212,7 +223,10 @@ class RetryInterceptorTests { AnnotatedClassBeanWithStrings proxy = ctx.getBean(AnnotatedClassBeanWithStrings.class); AnnotatedClassBeanWithStrings target = (AnnotatedClassBeanWithStrings) AopProxyUtils.getSingletonTarget(proxy); - assertThatIOException().isThrownBy(proxy::retryOperation).withMessage("3"); + // 3 = 1 initial invocation + 2 retry attempts + // Not 3 retry attempts, because RejectMalformedInputException3Predicate rejects + // a retry if the last exception was a MalformedInputException with message "3". + assertThatIOException().isThrownBy(proxy::retryOperation).withMessageContaining("3"); assertThat(target.counter).isEqualTo(3); assertThatIOException().isThrownBy(proxy::otherOperation); assertThat(target.counter).isEqualTo(4); @@ -237,7 +251,10 @@ class RetryInterceptorTests { AnnotatedClassBeanWithStrings proxy = ctx.getBean(AnnotatedClassBeanWithStrings.class); AnnotatedClassBeanWithStrings target = (AnnotatedClassBeanWithStrings) AopProxyUtils.getSingletonTarget(proxy); - assertThatIOException().isThrownBy(proxy::retryOperation).withMessage("3"); + // 3 = 1 initial invocation + 2 retry attempts + // Not 3 retry attempts, because RejectMalformedInputException3Predicate rejects + // a retry if the last exception was a MalformedInputException with message "3". + assertThatIOException().isThrownBy(proxy::retryOperation).withMessageContaining("3"); assertThat(target.counter).isEqualTo(3); assertThatIOException().isThrownBy(proxy::otherOperation); assertThat(target.counter).isEqualTo(4); @@ -267,6 +284,7 @@ class RetryInterceptorTests { int counter = 0; + @Override public void retryOperation() throws IOException { counter++; throw new IOException(Integer.toString(counter)); @@ -314,16 +332,24 @@ class RetryInterceptorTests { @Retryable(delay = 10, jitter = 5, multiplier = 2.0, maxDelay = 40, includes = IOException.class, excludes = AccessDeniedException.class, - predicate = CustomPredicate.class) + predicate = RejectMalformedInputException3Predicate.class) static class AnnotatedClassBean { int counter = 0; public void retryOperation() throws IOException { counter++; + if (counter == 3) { + throw new MalformedInputException(counter); + } throw new IOException(Integer.toString(counter)); } + public void retryOperationWithNestedException() { + counter++; + throw new RuntimeException(new IOException(Integer.toString(counter))); + } + public void otherOperation() throws IOException { counter++; throw new AccessDeniedException(Integer.toString(counter)); @@ -340,13 +366,16 @@ class RetryInterceptorTests { @Retryable(delayString = "${delay}", jitterString = "${jitter}", multiplierString = "${multiplier}", maxDelayString = "${maxDelay}", includes = IOException.class, excludes = AccessDeniedException.class, - predicate = CustomPredicate.class) + predicate = RejectMalformedInputException3Predicate.class) static class AnnotatedClassBeanWithStrings { int counter = 0; public void retryOperation() throws IOException { counter++; + if (counter == 3) { + throw new MalformedInputException(counter); + } throw new IOException(Integer.toString(counter)); } @@ -363,15 +392,6 @@ class RetryInterceptorTests { } - private static class CustomPredicate implements MethodRetryPredicate { - - @Override - public boolean shouldRetry(Method method, Throwable throwable) { - return !"3".equals(throwable.getMessage()); - } - } - - static class DoubleAnnotatedBean { AtomicInteger current = new AtomicInteger(); diff --git a/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java b/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java index 796718b34e7..703bce18e7a 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java +++ b/spring-core/src/main/java/org/springframework/core/retry/DefaultRetryPolicy.java @@ -58,7 +58,7 @@ class DefaultRetryPolicy implements RetryPolicy { @Override public boolean shouldRetry(Throwable throwable) { - return (this.exceptionFilter.match(throwable) && + return (this.exceptionFilter.match(throwable, true) && (this.predicate == null || this.predicate.test(throwable))); } diff --git a/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java b/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java index ea466e39ac7..eefe6c0ed52 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java +++ b/spring-core/src/main/java/org/springframework/core/retry/RetryPolicy.java @@ -296,6 +296,9 @@ public interface RetryPolicy { * Specify the types of exceptions for which the {@link RetryPolicy} * should retry a failed operation. *

Defaults to all exception types. + *

The supplied exception types will be matched against an exception + * thrown by a failed operation as well as nested + * {@linkplain Throwable#getCause() causes}. *

If included exception types have already been configured, the supplied * types will be added to the existing list of included types. *

This can be combined with other {@code includes}, {@code excludes}, @@ -318,6 +321,9 @@ public interface RetryPolicy { * Specify the types of exceptions for which the {@link RetryPolicy} * should retry a failed operation. *

Defaults to all exception types. + *

The supplied exception types will be matched against an exception + * thrown by a failed operation as well as nested + * {@linkplain Throwable#getCause() causes}. *

If included exception types have already been configured, the supplied * types will be added to the existing list of included types. *

This can be combined with other {@code includes}, {@code excludes}, @@ -337,6 +343,9 @@ public interface RetryPolicy { /** * Specify the types of exceptions for which the {@link RetryPolicy} * should not retry a failed operation. + *

The supplied exception types will be matched against an exception + * thrown by a failed operation as well as nested + * {@linkplain Throwable#getCause() causes}. *

If excluded exception types have already been configured, the supplied * types will be added to the existing list of excluded types. *

This can be combined with {@code includes}, other {@code excludes}, @@ -358,6 +367,9 @@ public interface RetryPolicy { /** * Specify the types of exceptions for which the {@link RetryPolicy} * should not retry a failed operation. + *

The supplied exception types will be matched against an exception + * thrown by a failed operation as well as nested + * {@linkplain Throwable#getCause() causes}. *

If excluded exception types have already been configured, the supplied * types will be added to the existing list of excluded types. *

This can be combined with {@code includes}, other {@code excludes}, diff --git a/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java b/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java index 42a92dea3ab..2194dd72ac3 100644 --- a/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java +++ b/spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java @@ -369,7 +369,7 @@ class RetryTemplateTests { public String execute() throws Exception { return switch (invocationCount.incrementAndGet()) { case 1 -> throw new IOException(); - case 2 -> throw new IOException(); + case 2 -> throw new RuntimeException(new IOException()); case 3 -> throw new CustomFileNotFoundException(); default -> "success"; }; @@ -388,14 +388,15 @@ class RetryTemplateTests { .withCauseExactlyInstanceOf(CustomFileNotFoundException.class) .satisfies(hasSuppressedExceptionsSatisfyingExactly( suppressed1 -> assertThat(suppressed1).isExactlyInstanceOf(IOException.class), - suppressed2 -> assertThat(suppressed2).isExactlyInstanceOf(IOException.class) + suppressed2 -> assertThat(suppressed2).isExactlyInstanceOf(RuntimeException.class) + .hasCauseExactlyInstanceOf(IOException.class) )) .satisfies(throwable -> assertThat(throwable.getRetryCount()).isEqualTo(2)) .satisfies(throwable -> { - repeat(2, () -> { - inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable); - inOrder.verify(retryListener).onRetryFailure(eq(retryPolicy), eq(retryable), any(IOException.class)); - }); + inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable); + inOrder.verify(retryListener).onRetryFailure(eq(retryPolicy), eq(retryable), any(RuntimeException.class)); + inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable); + inOrder.verify(retryListener).onRetryFailure(eq(retryPolicy), eq(retryable), any(CustomFileNotFoundException.class)); inOrder.verify(retryListener).onRetryPolicyExhaustion(retryPolicy, retryable, throwable); }); // 3 = 1 initial invocation + 2 retry attempts