Skip to content

Commit f606e6f

Browse files
committed
Fix retryLimit() to exclude Error types by default
When retryLimit() is used without explicit retry() configuration, the default behavior now retries Exception types only, excluding Error types like OutOfMemoryError and StackOverflowError. Resolves gh-5078 Signed-off-by: kjg <kimjg2477@gmail.com>
1 parent e0664f2 commit f606e6f

File tree

2 files changed

+159
-4
lines changed

2 files changed

+159
-4
lines changed

spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/ChunkOrientedStepBuilder.java

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,15 @@ public final ChunkOrientedStepBuilder<I, O> retry(Class<? extends Throwable>...
310310
return self();
311311
}
312312

313+
/**
314+
* Set the retry limit for the step. If no explicit retry exceptions are configured
315+
* via {@link #retry(Class[])}, the default is to retry all {@link Exception} types
316+
* but not {@link Error} types (e.g., OutOfMemoryError, StackOverflowError). This
317+
* ensures that fatal JVM errors fail immediately rather than being retried.
318+
* @param retryLimit the maximum number of retry attempts
319+
* @return this for fluent chaining
320+
* @since 6.0
321+
*/
313322
public ChunkOrientedStepBuilder<I, O> retryLimit(long retryLimit) {
314323
Assert.isTrue(retryLimit > 0, "retryLimit must be positive");
315324
this.retryLimit = retryLimit;
@@ -409,10 +418,13 @@ public ChunkOrientedStep<I, O> build() {
409418
chunkOrientedStep.setInterruptionPolicy(this.interruptionPolicy);
410419
if (this.retryPolicy == null) {
411420
if (!this.retryableExceptions.isEmpty() || this.retryLimit > 0) {
412-
this.retryPolicy = RetryPolicy.builder()
413-
.maxAttempts(this.retryLimit)
414-
.includes(this.retryableExceptions)
415-
.build();
421+
// Default to Exception.class when retryLimit is set without explicit
422+
// retry() configuration.
423+
// This prevents retrying fatal JVM errors like OutOfMemoryError and
424+
// StackOverflowError.
425+
Set<Class<? extends Throwable>> exceptions = this.retryableExceptions.isEmpty()
426+
? Set.of(Exception.class) : this.retryableExceptions;
427+
this.retryPolicy = RetryPolicy.builder().maxAttempts(this.retryLimit).includes(exceptions).build();
416428
}
417429
else {
418430
this.retryPolicy = throwable -> false;
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/*
2+
* Copyright 2025-present the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.batch.core.step.builder;
17+
18+
import java.util.ArrayList;
19+
import java.util.List;
20+
import java.util.concurrent.atomic.AtomicInteger;
21+
22+
import org.junit.jupiter.api.Test;
23+
24+
import org.springframework.batch.core.repository.JobRepository;
25+
import org.springframework.batch.core.step.item.ChunkOrientedStep;
26+
import org.springframework.batch.infrastructure.item.ItemProcessor;
27+
import org.springframework.batch.infrastructure.item.support.ListItemReader;
28+
import org.springframework.jdbc.support.JdbcTransactionManager;
29+
30+
import static org.junit.jupiter.api.Assertions.assertEquals;
31+
import static org.junit.jupiter.api.Assertions.assertThrows;
32+
import static org.mockito.Mockito.mock;
33+
34+
/**
35+
* Unit tests for {@link ChunkOrientedStepBuilder}.
36+
*
37+
* @since 6.0
38+
*/
39+
class ChunkOrientedStepBuilderTests {
40+
41+
@Test
42+
void testRetryLimitWithoutRetryDoesNotRetryErrors() {
43+
// Given: ItemProcessor that throws OutOfMemoryError
44+
AtomicInteger attempts = new AtomicInteger(0);
45+
ItemProcessor<String, String> processor = item -> {
46+
attempts.incrementAndGet();
47+
throw new OutOfMemoryError("Simulated OOM");
48+
};
49+
50+
ChunkOrientedStepBuilder<String, String> builder = new ChunkOrientedStepBuilder<>("testStep",
51+
mock(JobRepository.class), 2);
52+
builder.reader(new ListItemReader<>(List.of("item1"))).processor(processor).writer(items -> {
53+
}).transactionManager(mock(JdbcTransactionManager.class)).faultTolerant().retryLimit(3);
54+
55+
ChunkOrientedStep<String, String> step = builder.build();
56+
57+
// When & Then: Should fail immediately without retry
58+
// Currently this test FAILS (bug exists - Error is retried)
59+
// After fix: Should PASS (Error is not retried)
60+
assertThrows(Throwable.class, () -> {
61+
try {
62+
step.execute(null);
63+
}
64+
catch (Exception e) {
65+
throw e.getCause() != null ? e.getCause() : e;
66+
}
67+
});
68+
69+
// Bug: currently attempts.get() will be 4 (1 initial + 3 retries)
70+
// After fix: attempts.get() should be 1 (no retry)
71+
assertEquals(1, attempts.get(),
72+
"OutOfMemoryError should not be retried. Expected 1 attempt, but got " + attempts.get());
73+
}
74+
75+
@Test
76+
void testRetryLimitWithoutRetryRetriesExceptions() {
77+
// Given: ItemProcessor that fails first 2 times with Exception
78+
AtomicInteger attempts = new AtomicInteger(0);
79+
ItemProcessor<String, String> processor = item -> {
80+
if (attempts.incrementAndGet() < 3) {
81+
throw new RuntimeException("Temporary failure");
82+
}
83+
return item.toUpperCase();
84+
};
85+
86+
List<String> writtenItems = new ArrayList<>();
87+
ChunkOrientedStepBuilder<String, String> builder = new ChunkOrientedStepBuilder<>("testStep",
88+
mock(JobRepository.class), 2);
89+
builder.reader(new ListItemReader<>(List.of("item1")))
90+
.processor(processor)
91+
.writer(writtenItems::addAll)
92+
.transactionManager(mock(JdbcTransactionManager.class))
93+
.faultTolerant()
94+
.retryLimit(3);
95+
96+
ChunkOrientedStep<String, String> step = builder.build();
97+
98+
// When: Execute step
99+
// Then: Should succeed after 2 retries
100+
step.execute(null);
101+
102+
// Should have retried 2 times (total 3 attempts)
103+
assertEquals(3, attempts.get(), "Should retry RuntimeException");
104+
assertEquals(List.of("ITEM1"), writtenItems, "Item should be processed successfully");
105+
}
106+
107+
@Test
108+
void testExplicitRetryConfigurationTakesPrecedence() {
109+
// Given: Explicit retry configuration for IllegalStateException only
110+
AtomicInteger attempts = new AtomicInteger(0);
111+
ItemProcessor<String, String> processor = item -> {
112+
attempts.incrementAndGet();
113+
throw new RuntimeException("This should not be retried");
114+
};
115+
116+
ChunkOrientedStepBuilder<String, String> builder = new ChunkOrientedStepBuilder<>("testStep",
117+
mock(JobRepository.class), 2);
118+
builder.reader(new ListItemReader<>(List.of("item1"))).processor(processor).writer(items -> {
119+
})
120+
.transactionManager(mock(JdbcTransactionManager.class))
121+
.faultTolerant()
122+
.retry(IllegalStateException.class)
123+
.retryLimit(3);
124+
125+
ChunkOrientedStep<String, String> step = builder.build();
126+
127+
// When & Then: Should fail immediately without retry
128+
// because RuntimeException is not in the explicit retry list
129+
assertThrows(Throwable.class, () -> {
130+
try {
131+
step.execute(null);
132+
}
133+
catch (Exception e) {
134+
throw e.getCause() != null ? e.getCause() : e;
135+
}
136+
});
137+
138+
// Should not retry (only 1 attempt)
139+
assertEquals(1, attempts.get(),
140+
"RuntimeException should not be retried when only IllegalStateException is configured");
141+
}
142+
143+
}

0 commit comments

Comments
 (0)