diff --git a/spring-context/src/main/java/org/springframework/resilience/annotation/RetryAnnotationBeanPostProcessor.java b/spring-context/src/main/java/org/springframework/resilience/annotation/RetryAnnotationBeanPostProcessor.java index b30b11d36f7..4535d0ac9b1 100644 --- a/spring-context/src/main/java/org/springframework/resilience/annotation/RetryAnnotationBeanPostProcessor.java +++ b/spring-context/src/main/java/org/springframework/resilience/annotation/RetryAnnotationBeanPostProcessor.java @@ -30,6 +30,8 @@ import org.springframework.aop.framework.autoproxy.AbstractBeanFactoryAwareAdvis import org.springframework.aop.support.ComposablePointcut; import org.springframework.aop.support.DefaultPointcutAdvisor; import org.springframework.aop.support.annotation.AnnotationMatchingPointcut; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.context.ApplicationEventPublisherAware; import org.springframework.context.EmbeddedValueResolverAware; import org.springframework.core.MethodClassKey; import org.springframework.core.annotation.AnnotatedElementUtils; @@ -52,7 +54,9 @@ import org.springframework.util.StringValueResolver; */ @SuppressWarnings("serial") public class RetryAnnotationBeanPostProcessor extends AbstractBeanFactoryAwareAdvisingPostProcessor - implements EmbeddedValueResolverAware { + implements ApplicationEventPublisherAware, EmbeddedValueResolverAware { + + private final RetryAnnotationInterceptor interceptor = new RetryAnnotationInterceptor(); private @Nullable StringValueResolver embeddedValueResolver; @@ -62,9 +66,7 @@ public class RetryAnnotationBeanPostProcessor extends AbstractBeanFactoryAwareAd Pointcut cpc = new AnnotationMatchingPointcut(Retryable.class, true); Pointcut mpc = new AnnotationMatchingPointcut(null, Retryable.class, true); - this.advisor = new DefaultPointcutAdvisor( - new ComposablePointcut(cpc).union(mpc), - new RetryAnnotationInterceptor()); + this.advisor = new DefaultPointcutAdvisor(new ComposablePointcut(cpc).union(mpc), this.interceptor); } @@ -73,6 +75,11 @@ public class RetryAnnotationBeanPostProcessor extends AbstractBeanFactoryAwareAd this.embeddedValueResolver = resolver; } + @Override + public void setApplicationEventPublisher(ApplicationEventPublisher applicationEventPublisher) { + this.interceptor.setApplicationEventPublisher(applicationEventPublisher); + } + private class RetryAnnotationInterceptor extends AbstractRetryInterceptor { 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 6e556ecbafe..f86d69d6cc4 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 @@ -34,7 +34,10 @@ import org.springframework.resilience.retry.MethodRetryPredicate; * *

Aligned with {@link org.springframework.core.retry.RetryTemplate} * as well as Reactor's retry support, either re-invoking an imperative - * target method or decorating a reactive result accordingly. + * target method or decorating a returned reactive publisher accordingly. + * + *

For tracking the exceptions encountered by method-level retry processing, + * consider a {@link org.springframework.resilience.retry.MethodRetryEvent} listener. * *

Inspired by the Spring Retry * project but redesigned as a minimal core retry feature in the Spring Framework. diff --git a/spring-context/src/main/java/org/springframework/resilience/retry/AbstractRetryInterceptor.java b/spring-context/src/main/java/org/springframework/resilience/retry/AbstractRetryInterceptor.java index 6b3bc7f40a5..365ac18aeec 100644 --- a/spring-context/src/main/java/org/springframework/resilience/retry/AbstractRetryInterceptor.java +++ b/spring-context/src/main/java/org/springframework/resilience/retry/AbstractRetryInterceptor.java @@ -31,10 +31,14 @@ import reactor.core.publisher.Mono; import reactor.util.retry.Retry; import org.springframework.aop.ProxyMethodInvocation; +import org.springframework.context.ApplicationEventPublisher; +import org.springframework.context.ApplicationEventPublisherAware; import org.springframework.core.ReactiveAdapter; import org.springframework.core.ReactiveAdapterRegistry; import org.springframework.core.retry.RetryException; +import org.springframework.core.retry.RetryListener; import org.springframework.core.retry.RetryPolicy; +import org.springframework.core.retry.RetryState; import org.springframework.core.retry.RetryTemplate; import org.springframework.core.retry.Retryable; import org.springframework.util.ClassUtils; @@ -50,7 +54,7 @@ import org.springframework.util.ClassUtils; * @see Mono#retryWhen * @see Flux#retryWhen */ -public abstract class AbstractRetryInterceptor implements MethodInterceptor { +public abstract class AbstractRetryInterceptor implements MethodInterceptor, ApplicationEventPublisherAware { private static final Log logger = LogFactory.getLog(AbstractRetryInterceptor.class); @@ -62,6 +66,8 @@ public abstract class AbstractRetryInterceptor implements MethodInterceptor { private final @Nullable ReactiveAdapterRegistry reactiveAdapterRegistry; + private @Nullable ApplicationEventPublisher applicationEventPublisher; + public AbstractRetryInterceptor() { if (REACTIVE_STREAMS_PRESENT) { @@ -72,6 +78,11 @@ public abstract class AbstractRetryInterceptor implements MethodInterceptor { } } + @Override + public void setApplicationEventPublisher(ApplicationEventPublisher applicationEventPublisher) { + this.applicationEventPublisher = applicationEventPublisher; + } + @Override public @Nullable Object invoke(MethodInvocation invocation) throws Throwable { @@ -90,7 +101,7 @@ public abstract class AbstractRetryInterceptor implements MethodInterceptor { if (result == null) { return null; } - return ReactorDelegate.adaptReactiveResult(result, adapter, spec, method); + return new ReactorDelegate().adaptReactiveResult(invocation, result, adapter, spec); } } @@ -105,7 +116,17 @@ public abstract class AbstractRetryInterceptor implements MethodInterceptor { .multiplier(spec.multiplier()) .maxDelay(spec.maxDelay()) .build(); + RetryTemplate retryTemplate = new RetryTemplate(retryPolicy); + retryTemplate.setRetryListener(new RetryListener() { + @Override + public void onRetryableExecution(RetryPolicy retryPolicy, Retryable retryable, RetryState retryState) { + if (!retryState.isSuccessful()) { + onEvent(new MethodRetryEvent(invocation, retryState.getLastException(), false)); + } + } + }); + String methodName = ClassUtils.getQualifiedMethodName(method, (target != null ? target.getClass() : null)); try { @@ -122,13 +143,21 @@ public abstract class AbstractRetryInterceptor implements MethodInterceptor { }); } catch (RetryException ex) { + onEvent(new MethodRetryEvent(invocation, ex, true)); if (logger.isDebugEnabled()) { - logger.debug("@Retryable operation '%s' failed".formatted(methodName), ex); + logger.debug("Retryable operation '%s' failed".formatted(methodName), ex); } throw ex.getCause(); } } + private void onEvent(MethodRetryEvent event) { + logger.trace(event, event.getFailure()); + if (this.applicationEventPublisher != null) { + this.applicationEventPublisher.publishEvent(event); + } + } + /** * Determine the retry specification for the given method on the given target. * @param method the currently executing method @@ -141,29 +170,39 @@ public abstract class AbstractRetryInterceptor implements MethodInterceptor { /** * Inner class to avoid a hard dependency on Reactive Streams and Reactor at runtime. */ - private static class ReactorDelegate { + private class ReactorDelegate { - public static Object adaptReactiveResult( - Object result, ReactiveAdapter adapter, MethodRetrySpec spec, Method method) { + public Object adaptReactiveResult( + MethodInvocation invocation, Object result, ReactiveAdapter adapter, MethodRetrySpec spec) { Publisher publisher = adapter.toPublisher(result); Retry retry = Retry.backoff(spec.maxRetries(), spec.delay()) .jitter(calculateJitterFactor(spec)) .multiplier(spec.multiplier()) .maxBackoff(spec.maxDelay()) - .filter(spec.combinedPredicate().forMethod(method)); + .filter(spec.combinedPredicate().forMethod(invocation.getMethod())); Duration timeout = spec.timeout(); boolean timeoutIsPositive = (!timeout.isNegative() && !timeout.isZero()); if (adapter.isMultiValue()) { - publisher = (timeoutIsPositive ? - Flux.from(publisher).retryWhen(retry).timeout(timeout) : - Flux.from(publisher).retryWhen(retry)); + Flux flux = Flux.from(publisher) + .doOnError(ex -> onEvent(new MethodRetryEvent(invocation, ex, false))) + .retryWhen(retry); + if (timeoutIsPositive) { + flux = flux.timeout(timeout); + } + flux = flux.doOnError(ex -> onEvent(new MethodRetryEvent(invocation, ex, true))); + publisher = flux; } else { - publisher = (timeoutIsPositive ? - Mono.from(publisher).retryWhen(retry).timeout(timeout) : - Mono.from(publisher).retryWhen(retry)); + Mono mono = Mono.from(publisher) + .doOnError(ex -> onEvent(new MethodRetryEvent(invocation, ex, false))) + .retryWhen(retry); + if (timeoutIsPositive) { + mono = mono.timeout(timeout); + } + mono = mono.doOnError(ex -> onEvent(new MethodRetryEvent(invocation, ex, true))); + publisher = mono; } return adapter.fromPublisher(publisher); diff --git a/spring-context/src/main/java/org/springframework/resilience/retry/MethodRetryEvent.java b/spring-context/src/main/java/org/springframework/resilience/retry/MethodRetryEvent.java new file mode 100644 index 00000000000..8bbc3d1c097 --- /dev/null +++ b/spring-context/src/main/java/org/springframework/resilience/retry/MethodRetryEvent.java @@ -0,0 +1,110 @@ +/* + * 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.retry; + +import java.lang.reflect.Method; + +import org.aopalliance.intercept.MethodInvocation; + +import org.springframework.context.ApplicationEvent; +import org.springframework.util.ClassUtils; + +/** + * Event published for every exception encountered during retryable method invocation. + * Can be listened to via an {@code ApplicationListener} bean or an + * {@code @EventListener(MethodRetryEvent.class)} method. + * + * @author Juergen Hoeller + * @since 7.0.3 + * @see AbstractRetryInterceptor + * @see org.springframework.resilience.annotation.Retryable + * @see org.springframework.context.ApplicationListener + * @see org.springframework.context.event.EventListener + */ +@SuppressWarnings("serial") +public class MethodRetryEvent extends ApplicationEvent { + + private final Throwable failure; + + private final boolean retryAborted; + + + /** + * Create a new event for the given retryable method invocation. + * @param invocation the retryable method invocation + * @param failure the exception encountered + * @param retryAborted whether the current failure led to the retry execution getting aborted + */ + public MethodRetryEvent(MethodInvocation invocation, Throwable failure, boolean retryAborted) { + super(invocation); + this.failure = failure; + this.retryAborted = retryAborted; + } + + + /** + * Return the method invocation that triggered this event. + */ + @Override + public MethodInvocation getSource() { + return (MethodInvocation) super.getSource(); + } + + /** + * Return the method that triggered this event. + */ + public Method getMethod() { + return getSource().getMethod(); + } + + /** + * Return the exception encountered. + *

This may be an exception thrown by the method or emitted by the reactive + * publisher returned from the method, or a terminal exception on retry + * exhaustion, interruption or timeout. + *

For {@link org.springframework.core.retry.RetryTemplate} executions, + * an {@code instanceof RetryException} check identifies a final exception. + * For Reactor pipelines, {@code Exceptions.isRetryExhausted} identifies an + * exhaustion exception, whereas {@code instanceof TimeoutException} reveals + * a timeout scenario. + * @see #isRetryAborted() + * @see org.springframework.core.retry.RetryException + * @see reactor.core.Exceptions#isRetryExhausted + * @see java.util.concurrent.TimeoutException + */ + public Throwable getFailure() { + return this.failure; + } + + /** + * Return whether the current failure led to the retry execution getting aborted, + * typically indicating exhaustion, interruption or a timeout scenario. + *

If this returns {@code true}, {@link #getFailure()} exposes the final exception + * thrown by the retry infrastructure (rather than thrown by the method itself). + * @see #getFailure() + */ + public boolean isRetryAborted() { + return this.retryAborted; + } + + + @Override + public String toString() { + return "MethodRetryEvent: " + ClassUtils.getQualifiedMethodName(getMethod()) + " [" + getFailure() + "]"; + } + +} diff --git a/spring-context/src/test/java/org/springframework/resilience/MethodRetryEventListener.java b/spring-context/src/test/java/org/springframework/resilience/MethodRetryEventListener.java new file mode 100644 index 00000000000..df92db6ff71 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/resilience/MethodRetryEventListener.java @@ -0,0 +1,38 @@ +/* + * 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.util.ArrayList; +import java.util.List; + +import org.springframework.context.ApplicationListener; +import org.springframework.resilience.retry.MethodRetryEvent; + +/** + * @author Juergen Hoeller + * @since 7.0.3 + */ +class MethodRetryEventListener implements ApplicationListener { + + public final List events = new ArrayList<>(); + + @Override + public void onApplicationEvent(MethodRetryEvent event) { + this.events.add(event); + } + +} 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 152c74156bb..50bf4428094 100644 --- a/spring-context/src/test/java/org/springframework/resilience/ReactiveRetryInterceptorTests.java +++ b/spring-context/src/test/java/org/springframework/resilience/ReactiveRetryInterceptorTests.java @@ -17,6 +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; @@ -33,8 +34,11 @@ import reactor.core.publisher.Mono; import org.springframework.aop.framework.AopProxyUtils; import org.springframework.aop.framework.ProxyFactory; -import org.springframework.beans.factory.support.DefaultListableBeanFactory; +import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.support.RootBeanDefinition; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.support.GenericApplicationContext; +import org.springframework.resilience.annotation.EnableResilientMethods; import org.springframework.resilience.annotation.RetryAnnotationBeanPostProcessor; import org.springframework.resilience.annotation.Retryable; import org.springframework.resilience.retry.MethodRetrySpec; @@ -43,6 +47,7 @@ import org.springframework.resilience.retry.SimpleRetryInterceptor; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIllegalStateException; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.assertj.core.api.Assertions.assertThatRuntimeException; /** @@ -184,6 +189,52 @@ class ReactiveRetryInterceptorTests { assertThat(target.counter).hasValue(2); } + @Test + void withMethodRetryEventListener() throws Exception { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.registerBeanDefinition("bean", new RootBeanDefinition(AnnotatedMethodBean.class)); + ctx.registerBeanDefinition("config", new RootBeanDefinition(EnablingConfig.class)); + MethodRetryEventListener listener = new MethodRetryEventListener(); + ctx.addApplicationListener(listener); + ctx.refresh(); + AnnotatedMethodBean proxy = ctx.getBean(AnnotatedMethodBean.class); + AnnotatedMethodBean target = (AnnotatedMethodBean) AopProxyUtils.getSingletonTarget(proxy); + + Method method1 = AnnotatedMethodBean.class.getMethod("retryOperation"); + assertThatIllegalStateException() + .isThrownBy(() -> proxy.retryOperation().block()) + .satisfies(isRetryExhaustedException()); + assertThat(target.counter).hasValue(6); + assertThat(listener.events).hasSize(7); + for (int i = 0; i < 6; i++) { + String msg = Integer.toString(i + 1); + assertThat(listener.events.get(i)) + .satisfies(event -> assertThat(event.getMethod()).isEqualTo(method1)) + .satisfies(event -> assertThat(event.getFailure()).hasMessage(msg).isInstanceOf(IOException.class)) + .satisfies(event -> assertThat(event.isRetryAborted()).isFalse()); + } + assertThat(listener.events.get(6)) + .satisfies(event -> assertThat(event.getMethod()).isEqualTo(method1)) + .satisfies(event -> assertThat(event.getFailure()).satisfies(isRetryExhaustedException())) + .satisfies(event -> assertThat(event.isRetryAborted()).isTrue()); + + listener.events.clear(); + target.counter.set(0); + assertThatNoException().isThrownBy(() -> proxy.retryOperationWithInitialSuccess().block()); + assertThat(target.counter).hasValue(1); + assertThat(listener.events).isEmpty(); + + target.counter.set(0); + Method method2 = AnnotatedMethodBean.class.getMethod("retryOperationWithSuccessAfterInitialFailure"); + assertThatNoException().isThrownBy(() -> proxy.retryOperationWithSuccessAfterInitialFailure().block()); + assertThat(target.counter).hasValue(2); + assertThat(listener.events).hasSize(1); + assertThat(listener.events.get(0)) + .satisfies(event -> assertThat(event.getMethod()).isEqualTo(method2)) + .satisfies(event -> assertThat(event.getFailure()).hasMessage("1").isInstanceOf(IOException.class)) + .satisfies(event -> assertThat(event.isRetryAborted()).isFalse()); + } + @Test void adaptReactiveResultWithMinimalRetrySpec() { // Test minimal retry configuration: maxRetries=1, delay=0, jitter=0, multiplier=1.0, maxDelay=0 @@ -391,7 +442,6 @@ class ReactiveRetryInterceptorTests { // 1 initial attempt + 2 retries assertThat(target.counter).hasValue(3); } - } @@ -404,21 +454,29 @@ class ReactiveRetryInterceptorTests { } private static AnnotatedMethodBean getProxiedAnnotatedMethodBean() { - DefaultListableBeanFactory bf = createBeanFactoryFor(AnnotatedMethodBean.class); + BeanFactory bf = createBeanFactoryFor(AnnotatedMethodBean.class); return bf.getBean(AnnotatedMethodBean.class); } private static AnnotatedClassBean getProxiedAnnotatedClassBean() { - DefaultListableBeanFactory bf = createBeanFactoryFor(AnnotatedClassBean.class); + BeanFactory bf = createBeanFactoryFor(AnnotatedClassBean.class); return bf.getBean(AnnotatedClassBean.class); } - private static DefaultListableBeanFactory createBeanFactoryFor(Class beanClass) { + private static BeanFactory createBeanFactoryFor(Class beanClass) { + /* DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); bf.registerBeanDefinition("bean", new RootBeanDefinition(beanClass)); RetryAnnotationBeanPostProcessor bpp = new RetryAnnotationBeanPostProcessor(); bpp.setBeanFactory(bf); bf.addBeanPostProcessor(bpp); + */ + GenericApplicationContext bf = new GenericApplicationContext(); + bf.registerBeanDefinition("bean", new RootBeanDefinition(beanClass)); + bf.registerBeanDefinition("processor", new RootBeanDefinition(RetryAnnotationBeanPostProcessor.class)); + bf.registerBeanDefinition("listener", new RootBeanDefinition(MethodRetryEventListener.class)); + bf.refresh(); + return bf; } @@ -448,6 +506,24 @@ class ReactiveRetryInterceptorTests { }); } + @Retryable(maxRetries = 5, delay = 10) + public Mono retryOperationWithInitialSuccess() { + return Mono.fromCallable(() -> { + counter.incrementAndGet(); + return "success"; + }); + } + + @Retryable(maxRetries = 5, delay = 10) + public Mono retryOperationWithSuccessAfterInitialFailure() { + return Mono.fromCallable(() -> { + if (counter.incrementAndGet() == 1) { + throw new IOException(counter.toString()); + } + return "success"; + }); + } + @Retryable(timeout = 555, delay = 10) public Mono retryOperationWithTimeoutNotExceededAfterInitialSuccess() { return Mono.fromCallable(() -> { @@ -560,7 +636,9 @@ class ReactiveRetryInterceptorTests { } - + @EnableResilientMethods + static class EnablingConfig { + } // Bean classes for boundary testing 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 85a146b5254..79ae11979c3 100644 --- a/spring-context/src/test/java/org/springframework/resilience/RetryInterceptorTests.java +++ b/spring-context/src/test/java/org/springframework/resilience/RetryInterceptorTests.java @@ -18,6 +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; @@ -41,6 +42,7 @@ import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.support.GenericApplicationContext; import org.springframework.core.env.PropertiesPropertySource; +import org.springframework.core.retry.RetryException; import org.springframework.resilience.annotation.ConcurrencyLimit; import org.springframework.resilience.annotation.EnableResilientMethods; import org.springframework.resilience.annotation.RetryAnnotationBeanPostProcessor; @@ -53,6 +55,7 @@ import org.springframework.scheduling.annotation.EnableAsync; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; import static org.assertj.core.api.Assertions.assertThatIOException; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.assertj.core.api.Assertions.assertThatRuntimeException; /** @@ -312,6 +315,50 @@ class RetryInterceptorTests { assertThat(target.counter).hasValue(3); } + @Test + void withMethodRetryEventListener() throws Exception { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(); + ctx.registerBeanDefinition("bean", new RootBeanDefinition(AnnotatedMethodBean.class)); + ctx.registerBeanDefinition("config", new RootBeanDefinition(EnablingConfig.class)); + MethodRetryEventListener listener = new MethodRetryEventListener(); + ctx.addApplicationListener(listener); + ctx.refresh(); + AnnotatedMethodBean proxy = ctx.getBean(AnnotatedMethodBean.class); + AnnotatedMethodBean target = (AnnotatedMethodBean) AopProxyUtils.getSingletonTarget(proxy); + + Method method1 = AnnotatedMethodBean.class.getMethod("retryOperation"); + assertThatIOException().isThrownBy(proxy::retryOperation).withMessage("6"); + assertThat(target.counter).isEqualTo(6); + assertThat(listener.events).hasSize(7); + for (int i = 0; i < 6; i++) { + String msg = Integer.toString(i + 1); + assertThat(listener.events.get(i)) + .satisfies(event -> assertThat(event.getMethod()).isEqualTo(method1)) + .satisfies(event -> assertThat(event.getFailure()).hasMessage(msg).isInstanceOf(IOException.class)) + .satisfies(event -> assertThat(event.isRetryAborted()).isFalse()); + } + assertThat(listener.events.get(6)) + .satisfies(event -> assertThat(event.getMethod()).isEqualTo(method1)) + .satisfies(event -> assertThat(event.getFailure()).isInstanceOf(RetryException.class)) + .satisfies(event -> assertThat(event.isRetryAborted()).isTrue()); + + listener.events.clear(); + target.counter = 0; + assertThatNoException().isThrownBy(proxy::retryOperationWithInitialSuccess); + assertThat(target.counter).isEqualTo(1); + assertThat(listener.events).isEmpty(); + + target.counter = 0; + Method method2 = AnnotatedMethodBean.class.getMethod("retryOperationWithSuccessAfterInitialFailure"); + assertThatNoException().isThrownBy(proxy::retryOperationWithSuccessAfterInitialFailure); + assertThat(target.counter).isEqualTo(2); + assertThat(listener.events).hasSize(1); + assertThat(listener.events.get(0)) + .satisfies(event -> assertThat(event.getMethod()).isEqualTo(method2)) + .satisfies(event -> assertThat(event.getFailure()).hasMessage("1").isInstanceOf(IOException.class)) + .satisfies(event -> assertThat(event.isRetryAborted()).isFalse()); + } + @Nested class TimeoutTests { @@ -372,7 +419,6 @@ class RetryInterceptorTests { // 1 initial attempt + 2 retries assertThat(target.counter).isEqualTo(3); } - } @@ -414,6 +460,20 @@ class RetryInterceptorTests { throw new IOException(Integer.toString(counter)); } + @Retryable(maxRetries = 5, delay = 10) + public String retryOperationWithInitialSuccess() { + counter++; + return "success"; + } + + @Retryable(maxRetries = 5, delay = 10) + public String retryOperationWithSuccessAfterInitialFailure() throws IOException{ + if (++counter == 1) { + throw new IOException(Integer.toString(counter)); + } + return "success"; + } + @Retryable(timeout = 555, delay = 10) public String retryOperationWithTimeoutNotExceededAfterInitialSuccess() { counter++; 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 5c295436031..b664c7961f2 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 @@ -219,6 +219,9 @@ public interface RetryPolicy { * invoked at least once and at most 5 times. *

The default is {@value #DEFAULT_MAX_RETRIES}. *

The supplied value will override any previously configured value. + * Note that {@link RetryTemplate} effectively only supports an integer + * range since it stores all exceptions, so it will always exhaust at + * {@code Integer#MAX_VALUE} even if a larger value is specified here. *

You should not specify this configuration option if you have * configured a custom {@link #backOff(BackOff) BackOff} strategy. * @param maxRetries the maximum number of retry attempts; diff --git a/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java b/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java index ba89a5417b6..ab0b9a54bc3 100644 --- a/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java +++ b/spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java @@ -146,7 +146,7 @@ public class RetryTemplate implements RetryOperations { Throwable lastException = initialException; long timeout = this.retryPolicy.getTimeout().toMillis(); - while (this.retryPolicy.shouldRetry(lastException)) { + while (this.retryPolicy.shouldRetry(lastException) && retryState.getRetryCount() < Integer.MAX_VALUE) { checkIfTimeoutExceeded(timeout, startTime, 0, retryable, retryState); try { @@ -161,9 +161,8 @@ public class RetryTemplate implements RetryOperations { } catch (InterruptedException interruptedException) { Thread.currentThread().interrupt(); - RetryException retryException = new RetryException( - "Interrupted during back-off for retryable operation '%s'".formatted(retryableName), - retryState); + RetryException retryException = new RetryException("Interrupted during back-off for " + + "retryable operation '%s'; aborting execution".formatted(retryableName), retryState); this.retryListener.onRetryPolicyInterruption(this.retryPolicy, retryable, retryException); throw retryException; } 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 775855dcace..000d5f63105 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 @@ -258,7 +258,7 @@ class RetryTemplateTests { assertThatExceptionOfType(RetryException.class) .isThrownBy(() -> retryTemplate.execute(retryable)) - .withMessageMatching("Interrupted during back-off for retryable operation '.+?'") + .withMessageMatching("Interrupted during back-off for retryable operation '.+?'; aborting execution") .withCause(exception) .satisfies(throwable -> assertThat(throwable.getSuppressed()).isEmpty()) .satisfies(throwable -> assertThat(throwable.getRetryCount()).isZero())