Skip to content

Commit c386066

Browse files
committed
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
1 parent 9982369 commit c386066

File tree

2 files changed

+73
-10
lines changed

2 files changed

+73
-10
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ public void setRetryListener(RetryListener retryListener) {
174174
"Retry policy for operation '%s' exhausted; aborting execution".formatted(retryableName),
175175
exceptions.removeLast());
176176
exceptions.forEach(finalException::addSuppressed);
177-
this.retryListener.onRetryPolicyExhaustion(this.retryPolicy, retryable, finalException);
177+
this.retryListener.onRetryPolicyExhaustion(this.retryPolicy, retryable, retryException);
178178
throw finalException;
179179
}
180180
}

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

Lines changed: 72 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,20 @@
2929
import org.junit.jupiter.params.ParameterizedTest;
3030
import org.junit.jupiter.params.provider.Arguments.ArgumentSet;
3131
import org.junit.jupiter.params.provider.FieldSource;
32+
import org.mockito.InOrder;
3233

3334
import static org.assertj.core.api.Assertions.assertThat;
3435
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
3536
import static org.junit.jupiter.params.provider.Arguments.argumentSet;
37+
import static org.mockito.ArgumentMatchers.any;
38+
import static org.mockito.ArgumentMatchers.eq;
39+
import static org.mockito.Mockito.inOrder;
40+
import static org.mockito.Mockito.mock;
41+
import static org.mockito.Mockito.verifyNoInteractions;
3642

3743
/**
38-
* Integration tests for {@link RetryTemplate} and {@link RetryPolicy}.
44+
* Integration tests for {@link RetryTemplate}, {@link RetryPolicy} and
45+
* {@link RetryListener}.
3946
*
4047
* @author Mahmoud Ben Hassine
4148
* @author Sam Brannen
@@ -44,17 +51,22 @@
4451
*/
4552
class RetryTemplateTests {
4653

47-
private final RetryTemplate retryTemplate = new RetryTemplate();
48-
49-
50-
@BeforeEach
51-
void configureRetryTemplate() {
52-
var retryPolicy = RetryPolicy.builder()
54+
private final RetryPolicy retryPolicy =
55+
RetryPolicy.builder()
5356
.maxAttempts(3)
5457
.delay(Duration.ZERO)
5558
.build();
5659

57-
retryTemplate.setRetryPolicy(retryPolicy);
60+
private final RetryTemplate retryTemplate = new RetryTemplate(retryPolicy);
61+
62+
private final RetryListener retryListener = mock();
63+
64+
private final InOrder inOrder = inOrder(retryListener);
65+
66+
67+
@BeforeEach
68+
void configureRetryTemplate() {
69+
retryTemplate.setRetryListener(retryListener);
5870
}
5971

6072
@Test
@@ -68,21 +80,32 @@ void retryWithImmediateSuccess() throws Exception {
6880
assertThat(invocationCount).hasValue(0);
6981
assertThat(retryTemplate.execute(retryable)).isEqualTo("always succeeds");
7082
assertThat(invocationCount).hasValue(1);
83+
84+
// RetryListener interactions:
85+
verifyNoInteractions(retryListener);
7186
}
7287

7388
@Test
7489
void retryWithSuccessAfterInitialFailures() throws Exception {
90+
Exception exception = new Exception("Boom!");
7591
AtomicInteger invocationCount = new AtomicInteger();
7692
Retryable<String> retryable = () -> {
7793
if (invocationCount.incrementAndGet() <= 2) {
78-
throw new Exception("Boom!");
94+
throw exception;
7995
}
8096
return "finally succeeded";
8197
};
8298

8399
assertThat(invocationCount).hasValue(0);
84100
assertThat(retryTemplate.execute(retryable)).isEqualTo("finally succeeded");
85101
assertThat(invocationCount).hasValue(3);
102+
103+
// RetryListener interactions:
104+
inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable);
105+
inOrder.verify(retryListener).onRetryFailure(retryPolicy, retryable, exception);
106+
inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable);
107+
inOrder.verify(retryListener).onRetrySuccess(retryPolicy, retryable, "finally succeeded");
108+
inOrder.verifyNoMoreInteractions();
86109
}
87110

88111
@Test
@@ -110,6 +133,14 @@ public String getName() {
110133
.withCause(exception);
111134
// 4 = 1 initial invocation + 3 retry attempts
112135
assertThat(invocationCount).hasValue(4);
136+
137+
// RetryListener interactions:
138+
repeat(3, () -> {
139+
inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable);
140+
inOrder.verify(retryListener).onRetryFailure(retryPolicy, retryable, exception);
141+
});
142+
inOrder.verify(retryListener).onRetryPolicyExhaustion(retryPolicy, retryable, exception);
143+
inOrder.verifyNoMoreInteractions();
113144
}
114145

115146
@Test
@@ -146,6 +177,14 @@ public String getName() {
146177
.withCause(exception);
147178
// 6 = 1 initial invocation + 5 retry attempts
148179
assertThat(invocationCount).hasValue(6);
180+
181+
// RetryListener interactions:
182+
repeat(5, () -> {
183+
inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable);
184+
inOrder.verify(retryListener).onRetryFailure(retryPolicy, retryable, exception);
185+
});
186+
inOrder.verify(retryListener).onRetryPolicyExhaustion(retryPolicy, retryable, exception);
187+
inOrder.verifyNoMoreInteractions();
149188
}
150189

151190
@Test
@@ -188,6 +227,15 @@ public String getName() {
188227
));
189228
// 3 = 1 initial invocation + 2 retry attempts
190229
assertThat(invocationCount).hasValue(3);
230+
231+
// RetryListener interactions:
232+
repeat(2, () -> {
233+
inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable);
234+
inOrder.verify(retryListener).onRetryFailure(eq(retryPolicy), eq(retryable), any(Throwable.class));
235+
});
236+
inOrder.verify(retryListener).onRetryPolicyExhaustion(
237+
eq(retryPolicy), eq(retryable), any(IllegalStateException.class));
238+
inOrder.verifyNoMoreInteractions();
191239
}
192240

193241
static final List<ArgumentSet> includesAndExcludesRetryPolicies = List.of(
@@ -241,9 +289,24 @@ public String getName() {
241289
));
242290
// 3 = 1 initial invocation + 2 retry attempts
243291
assertThat(invocationCount).hasValue(3);
292+
293+
// RetryListener interactions:
294+
repeat(2, () -> {
295+
inOrder.verify(retryListener).beforeRetry(retryPolicy, retryable);
296+
inOrder.verify(retryListener).onRetryFailure(eq(retryPolicy), eq(retryable), any(Throwable.class));
297+
});
298+
inOrder.verify(retryListener).onRetryPolicyExhaustion(
299+
eq(retryPolicy), eq(retryable), any(CustomFileNotFoundException.class));
300+
inOrder.verifyNoMoreInteractions();
244301
}
245302

246303

304+
private static void repeat(int times, Runnable runnable) {
305+
for (int i = 0; i < times; i++) {
306+
runnable.run();
307+
}
308+
}
309+
247310
@SafeVarargs
248311
private static final Consumer<Throwable> hasSuppressedExceptionsSatisfyingExactly(
249312
ThrowingConsumer<? super Throwable>... requirements) {

0 commit comments

Comments
 (0)