root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 2 Packages Projects Releases Wiki Activity

Supply correct exception to RetryListener.onRetryPolicyExhaustion() 

Prior to this commit, RetryTemplate supplied the wrong exception to
RetryListener.onRetryPolicyExhaustion(). Specifically, the execute()
method in RetryTemplate supplied the final, composite RetryException to
onRetryPolicyExhaustion() instead of the last exception thrown by the
Retryable operation.

This commit fixes that bug by ensuring that the last exception thrown by
the Retryable operation is supplied to onRetryPolicyExhaustion().

Closes gh-35334
This commit is contained in:
Sam Brannen 2025-08-16 16:07:37 +02:00
parent 9982369982
commit c38606610c
2 changed files with 73 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
spring-core/src/main/java/org/springframework/core/retry/RetryTemplate.java
View File

@ -174,7 +174,7 @@ public class RetryTemplate implements RetryOperations {
"Retry policy for operation '%s' exhausted; aborting execution".formatted(retryableName),
exceptions.removeLast());
exceptions.forEach(finalException::addSuppressed);
this.retryListener.onRetryPolicyExhaustion(this.retryPolicy, retryable, finalException);
this.retryListener.onRetryPolicyExhaustion(this.retryPolicy, retryable, retryException);
throw finalException;
}
}

81
spring-core/src/test/java/org/springframework/core/retry/RetryTemplateTests.java
View File

@ -29,13 +29,20 @@ import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments.ArgumentSet;
import org.junit.jupiter.params.provider.FieldSource;
import org.mockito.InOrder;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.junit.jupiter.params.provider.Arguments.argumentSet;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verifyNoInteractions;
/**
* Integration tests for {@link RetryTemplate} and {@link RetryPolicy}.
* Integration tests for {@link RetryTemplate}, {@link RetryPolicy} and
* {@link RetryListener}.
*
* @author Mahmoud Ben Hassine
* @author Sam Brannen
@ -44,17 +51,22 @@ import static org.junit.jupiter.params.provider.Arguments.argumentSet;
*/
class RetryTemplateTests {
private final RetryTemplate retryTemplate = new RetryTemplate();
@BeforeEach
void configureRetryTemplate() {
var retryPolicy = RetryPolicy.builder()
private final RetryPolicy retryPolicy =
RetryPolicy.builder()
.maxAttempts(3)
.delay(Duration.ZERO)
.build();
retryTemplate.setRetryPolicy(retryPolicy);
private final RetryTemplate retryTemplate = new RetryTemplate(retryPolicy);
private final RetryListener retryListener = mock();
private final InOrder inOrder = inOrder(retryListener);
@BeforeEach
void configureRetryTemplate() {
retryTemplate.setRetryListener(retryListener);
}
@Test
@ -68,14 +80,18 @@ class RetryTemplateTests {
assertThat(invocationCount).hasValue(0);
assertThat(retryTemplate.execute(retryable)).isEqualTo("always succeeds");
assertThat(invocationCount).hasValue(1);
// RetryListener interactions:
verifyNoInteractions(retryListener);
}
@Test
void retryWithSuccessAfterInitialFailures() throws Exception {
Exception exception = new Exception("Boom!");
AtomicInteger invocationCount = new AtomicInteger();
Retryable<String> retryable = () -> {
if (invocationCount.incrementAndGet() <= 2) {
throw new Exception("Boom!");
throw exception;
}
return "finally succeeded";
};
@ -83,6 +99,13 @@ class RetryTemplateTests {
assertThat(invocationCount).hasValue(0);
assertThat(retryTemplate.execute(retryable)).isEqualTo("finally succeeded");
assertThat(invocationCount).hasValue(3);
// RetryListener interactions:
inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable);
inOrder.verify(retryListener).onRetryFailure(retryPolicy, retryable, exception);
inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable);
inOrder.verify(retryListener).onRetrySuccess(retryPolicy, retryable, "finally succeeded");
inOrder.verifyNoMoreInteractions();
}
@Test
@ -110,6 +133,14 @@ class RetryTemplateTests {
.withCause(exception);
// 4 = 1 initial invocation + 3 retry attempts
assertThat(invocationCount).hasValue(4);
// RetryListener interactions:
repeat(3, () -> {
inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable);
inOrder.verify(retryListener).onRetryFailure(retryPolicy, retryable, exception);
});
inOrder.verify(retryListener).onRetryPolicyExhaustion(retryPolicy, retryable, exception);
inOrder.verifyNoMoreInteractions();
}
@Test
@ -146,6 +177,14 @@ class RetryTemplateTests {
.withCause(exception);
// 6 = 1 initial invocation + 5 retry attempts
assertThat(invocationCount).hasValue(6);
// RetryListener interactions:
repeat(5, () -> {
inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable);
inOrder.verify(retryListener).onRetryFailure(retryPolicy, retryable, exception);
});
inOrder.verify(retryListener).onRetryPolicyExhaustion(retryPolicy, retryable, exception);
inOrder.verifyNoMoreInteractions();
}
@Test
@ -188,6 +227,15 @@ class RetryTemplateTests {
));
// 3 = 1 initial invocation + 2 retry attempts
assertThat(invocationCount).hasValue(3);
// RetryListener interactions:
repeat(2, () -> {
inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable);
inOrder.verify(retryListener).onRetryFailure(eq(retryPolicy), eq(retryable), any(Throwable.class));
});
inOrder.verify(retryListener).onRetryPolicyExhaustion(
eq(retryPolicy), eq(retryable), any(IllegalStateException.class));
inOrder.verifyNoMoreInteractions();
}
static final List<ArgumentSet> includesAndExcludesRetryPolicies = List.of(
@ -241,9 +289,24 @@ class RetryTemplateTests {
));
// 3 = 1 initial invocation + 2 retry attempts
assertThat(invocationCount).hasValue(3);
// RetryListener interactions:
repeat(2, () -> {
inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable);
inOrder.verify(retryListener).onRetryFailure(eq(retryPolicy), eq(retryable), any(Throwable.class));
});
inOrder.verify(retryListener).onRetryPolicyExhaustion(
eq(retryPolicy), eq(retryable), any(CustomFileNotFoundException.class));
inOrder.verifyNoMoreInteractions();
}
private static void repeat(int times, Runnable runnable) {
for (int i = 0; i < times; i++) {
runnable.run();
}
}
@SafeVarargs
private static final Consumer<Throwable> hasSuppressedExceptionsSatisfyingExactly(
ThrowingConsumer<? super Throwable>... requirements) {