From f606e6f31c9ce6334183384485f14422e124a685 Mon Sep 17 00:00:00 2001 From: kjg Date: Sat, 8 Nov 2025 13:18:16 +0900 Subject: [PATCH] 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 --- .../builder/ChunkOrientedStepBuilder.java | 20 ++- .../ChunkOrientedStepBuilderTests.java | 143 ++++++++++++++++++ 2 files changed, 159 insertions(+), 4 deletions(-) create mode 100644 spring-batch-core/src/test/java/org/springframework/batch/core/step/builder/ChunkOrientedStepBuilderTests.java diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/ChunkOrientedStepBuilder.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/ChunkOrientedStepBuilder.java index 5942265d11..6d6825a2c5 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/ChunkOrientedStepBuilder.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/builder/ChunkOrientedStepBuilder.java @@ -310,6 +310,15 @@ public final ChunkOrientedStepBuilder retry(Class... return self(); } + /** + * Set the retry limit for the step. If no explicit retry exceptions are configured + * via {@link #retry(Class[])}, the default is to retry all {@link Exception} types + * but not {@link Error} types (e.g., OutOfMemoryError, StackOverflowError). This + * ensures that fatal JVM errors fail immediately rather than being retried. + * @param retryLimit the maximum number of retry attempts + * @return this for fluent chaining + * @since 6.0 + */ public ChunkOrientedStepBuilder retryLimit(long retryLimit) { Assert.isTrue(retryLimit > 0, "retryLimit must be positive"); this.retryLimit = retryLimit; @@ -409,10 +418,13 @@ public ChunkOrientedStep build() { chunkOrientedStep.setInterruptionPolicy(this.interruptionPolicy); if (this.retryPolicy == null) { if (!this.retryableExceptions.isEmpty() || this.retryLimit > 0) { - this.retryPolicy = RetryPolicy.builder() - .maxAttempts(this.retryLimit) - .includes(this.retryableExceptions) - .build(); + // Default to Exception.class when retryLimit is set without explicit + // retry() configuration. + // This prevents retrying fatal JVM errors like OutOfMemoryError and + // StackOverflowError. + Set> exceptions = this.retryableExceptions.isEmpty() + ? Set.of(Exception.class) : this.retryableExceptions; + this.retryPolicy = RetryPolicy.builder().maxAttempts(this.retryLimit).includes(exceptions).build(); } else { this.retryPolicy = throwable -> false; diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/builder/ChunkOrientedStepBuilderTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/builder/ChunkOrientedStepBuilderTests.java new file mode 100644 index 0000000000..8e38a58c77 --- /dev/null +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/builder/ChunkOrientedStepBuilderTests.java @@ -0,0 +1,143 @@ +/* + * Copyright 2025-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.batch.core.step.builder; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.jupiter.api.Test; + +import org.springframework.batch.core.repository.JobRepository; +import org.springframework.batch.core.step.item.ChunkOrientedStep; +import org.springframework.batch.infrastructure.item.ItemProcessor; +import org.springframework.batch.infrastructure.item.support.ListItemReader; +import org.springframework.jdbc.support.JdbcTransactionManager; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; + +/** + * Unit tests for {@link ChunkOrientedStepBuilder}. + * + * @since 6.0 + */ +class ChunkOrientedStepBuilderTests { + + @Test + void testRetryLimitWithoutRetryDoesNotRetryErrors() { + // Given: ItemProcessor that throws OutOfMemoryError + AtomicInteger attempts = new AtomicInteger(0); + ItemProcessor processor = item -> { + attempts.incrementAndGet(); + throw new OutOfMemoryError("Simulated OOM"); + }; + + ChunkOrientedStepBuilder builder = new ChunkOrientedStepBuilder<>("testStep", + mock(JobRepository.class), 2); + builder.reader(new ListItemReader<>(List.of("item1"))).processor(processor).writer(items -> { + }).transactionManager(mock(JdbcTransactionManager.class)).faultTolerant().retryLimit(3); + + ChunkOrientedStep step = builder.build(); + + // When & Then: Should fail immediately without retry + // Currently this test FAILS (bug exists - Error is retried) + // After fix: Should PASS (Error is not retried) + assertThrows(Throwable.class, () -> { + try { + step.execute(null); + } + catch (Exception e) { + throw e.getCause() != null ? e.getCause() : e; + } + }); + + // Bug: currently attempts.get() will be 4 (1 initial + 3 retries) + // After fix: attempts.get() should be 1 (no retry) + assertEquals(1, attempts.get(), + "OutOfMemoryError should not be retried. Expected 1 attempt, but got " + attempts.get()); + } + + @Test + void testRetryLimitWithoutRetryRetriesExceptions() { + // Given: ItemProcessor that fails first 2 times with Exception + AtomicInteger attempts = new AtomicInteger(0); + ItemProcessor processor = item -> { + if (attempts.incrementAndGet() < 3) { + throw new RuntimeException("Temporary failure"); + } + return item.toUpperCase(); + }; + + List writtenItems = new ArrayList<>(); + ChunkOrientedStepBuilder builder = new ChunkOrientedStepBuilder<>("testStep", + mock(JobRepository.class), 2); + builder.reader(new ListItemReader<>(List.of("item1"))) + .processor(processor) + .writer(writtenItems::addAll) + .transactionManager(mock(JdbcTransactionManager.class)) + .faultTolerant() + .retryLimit(3); + + ChunkOrientedStep step = builder.build(); + + // When: Execute step + // Then: Should succeed after 2 retries + step.execute(null); + + // Should have retried 2 times (total 3 attempts) + assertEquals(3, attempts.get(), "Should retry RuntimeException"); + assertEquals(List.of("ITEM1"), writtenItems, "Item should be processed successfully"); + } + + @Test + void testExplicitRetryConfigurationTakesPrecedence() { + // Given: Explicit retry configuration for IllegalStateException only + AtomicInteger attempts = new AtomicInteger(0); + ItemProcessor processor = item -> { + attempts.incrementAndGet(); + throw new RuntimeException("This should not be retried"); + }; + + ChunkOrientedStepBuilder builder = new ChunkOrientedStepBuilder<>("testStep", + mock(JobRepository.class), 2); + builder.reader(new ListItemReader<>(List.of("item1"))).processor(processor).writer(items -> { + }) + .transactionManager(mock(JdbcTransactionManager.class)) + .faultTolerant() + .retry(IllegalStateException.class) + .retryLimit(3); + + ChunkOrientedStep step = builder.build(); + + // When & Then: Should fail immediately without retry + // because RuntimeException is not in the explicit retry list + assertThrows(Throwable.class, () -> { + try { + step.execute(null); + } + catch (Exception e) { + throw e.getCause() != null ? e.getCause() : e; + } + }); + + // Should not retry (only 1 attempt) + assertEquals(1, attempts.get(), + "RuntimeException should not be retried when only IllegalStateException is configured"); + } + +}