RESOLVED - issue BATCH-1354: Infinite loop caused by throwing an Error from the ItemWriter of a skippable step
http://jira.springframework.org/browse/BATCH-1354 Previous change reverted: Error should be fatal, so it has to be added to the exception classifiers in FaultTolerantStepFactoryBean.
This commit is contained in:
@@ -283,11 +283,8 @@ public class FaultTolerantChunkProcessor<I, O> extends SimpleChunkProcessor<I, O
|
||||
try {
|
||||
doWrite(outputs.getItems());
|
||||
}
|
||||
catch (Throwable t) {
|
||||
Exception e = (t instanceof Exception) ? (Exception) t : new IllegalStateException(
|
||||
"Unexpected non-Exception Throwable", t);
|
||||
|
||||
if (rollbackClassifier.classify(t)) {
|
||||
catch (Exception e) {
|
||||
if (rollbackClassifier.classify(e)) {
|
||||
throw e;
|
||||
}
|
||||
/*
|
||||
@@ -315,7 +312,7 @@ public class FaultTolerantChunkProcessor<I, O> extends SimpleChunkProcessor<I, O
|
||||
|
||||
public Object recover(RetryContext context) throws Exception {
|
||||
|
||||
Exception e = context.getLastThrowable();
|
||||
Exception e = (Exception) context.getLastThrowable();
|
||||
if (outputs.size() > 1 && !rollbackClassifier.classify(e)) {
|
||||
throw new RetryException("Invalid retry state during write caused by "
|
||||
+ "exception that does not classify for rollback: ", e);
|
||||
@@ -387,7 +384,7 @@ public class FaultTolerantChunkProcessor<I, O> extends SimpleChunkProcessor<I, O
|
||||
if (item == null) {
|
||||
continue;
|
||||
}
|
||||
Throwable e = wrapper.getException();
|
||||
Exception e = wrapper.getException();
|
||||
try {
|
||||
getListener().onSkipInProcess(item, e);
|
||||
}
|
||||
@@ -397,7 +394,7 @@ public class FaultTolerantChunkProcessor<I, O> extends SimpleChunkProcessor<I, O
|
||||
}
|
||||
|
||||
for (SkipWrapper<O> wrapper : outputs.getSkips()) {
|
||||
Throwable e = wrapper.getException();
|
||||
Exception e = wrapper.getException();
|
||||
try {
|
||||
getListener().onSkipInWrite(wrapper.getItem(), e);
|
||||
}
|
||||
@@ -467,20 +464,16 @@ public class FaultTolerantChunkProcessor<I, O> extends SimpleChunkProcessor<I, O
|
||||
inputIterator.remove();
|
||||
outputIterator.remove();
|
||||
}
|
||||
catch (Throwable t) {
|
||||
|
||||
Exception e = (t instanceof Exception) ? (Exception) t : new IllegalStateException(
|
||||
"Unexpected non-Exception Throwable", t);
|
||||
|
||||
if (!itemWriteSkipPolicy.shouldSkip(e, -1) && !rollbackClassifier.classify(t)) {
|
||||
catch (Exception e) {
|
||||
if (!itemWriteSkipPolicy.shouldSkip(e, -1) && !rollbackClassifier.classify(e)) {
|
||||
inputIterator.remove();
|
||||
outputIterator.remove();
|
||||
}
|
||||
else {
|
||||
checkSkipPolicy(inputIterator, outputIterator, e, contribution);
|
||||
}
|
||||
if (rollbackClassifier.classify(t)) {
|
||||
throw e;
|
||||
if (rollbackClassifier.classify(e)) {
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
chunkMonitor.incrementOffset();
|
||||
|
||||
@@ -300,9 +300,9 @@ public class FaultTolerantStepFactoryBean<T, S> extends SimpleStepFactoryBean<T,
|
||||
protected void applyConfiguration(TaskletStep step) {
|
||||
|
||||
addFatalExceptionIfMissing(SkipLimitExceededException.class, NonSkippableReadException.class,
|
||||
SkipListenerFailedException.class, RetryException.class, JobInterruptedException.class);
|
||||
SkipListenerFailedException.class, RetryException.class, JobInterruptedException.class, Error.class);
|
||||
addNonRetryableExceptionIfMissing(SkipLimitExceededException.class, NonSkippableReadException.class,
|
||||
SkipListenerFailedException.class, RetryException.class, JobInterruptedException.class);
|
||||
SkipListenerFailedException.class, RetryException.class, JobInterruptedException.class, Error.class);
|
||||
|
||||
super.applyConfiguration(step);
|
||||
|
||||
|
||||
@@ -34,7 +34,7 @@ public abstract class AbstractExceptionThrowingItemHandlerStub<T> {
|
||||
|
||||
private Collection<T> failures = Collections.emptyList();
|
||||
|
||||
private Constructor<? extends Exception> exception;
|
||||
private Constructor<? extends Throwable> exception;
|
||||
|
||||
public AbstractExceptionThrowingItemHandlerStub() throws Exception {
|
||||
exception = SkippableRuntimeException.class.getConstructor(String.class);
|
||||
@@ -44,8 +44,12 @@ public abstract class AbstractExceptionThrowingItemHandlerStub<T> {
|
||||
this.failures = new ArrayList<T>(Arrays.asList(failures));
|
||||
}
|
||||
|
||||
public void setExceptionType(Class<? extends Exception> exceptionType) throws Exception {
|
||||
exception = exceptionType.getConstructor(String.class);
|
||||
public void setExceptionType(Class<? extends Throwable> exceptionType) throws Exception {
|
||||
try {
|
||||
exception = exceptionType.getConstructor(String.class);
|
||||
} catch (NoSuchMethodException e) {
|
||||
exception = exceptionType.getConstructor(Object.class);
|
||||
}
|
||||
}
|
||||
|
||||
public void clearFailures() {
|
||||
@@ -54,7 +58,14 @@ public abstract class AbstractExceptionThrowingItemHandlerStub<T> {
|
||||
|
||||
protected void checkFailure(T item) throws Exception {
|
||||
if (isFailure(item)) {
|
||||
throw exception.newInstance("Intended Failure: " + item);
|
||||
Throwable t = exception.newInstance("Intended Failure: " + item);
|
||||
if (t instanceof Exception) {
|
||||
throw (Exception) t;
|
||||
}
|
||||
if (t instanceof Error) {
|
||||
throw (Error) t;
|
||||
}
|
||||
throw new IllegalStateException("Unexpected non-Error Throwable");
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -98,7 +98,11 @@ public class FaultTolerantChunkProcessorTests {
|
||||
assertEquals(1, contribution.getFilterCount());
|
||||
}
|
||||
|
||||
@Test
|
||||
/**
|
||||
* An Error pops right back up (no skips, no retry)
|
||||
* @throws Exception
|
||||
*/
|
||||
@Test(expected=AssertionError.class)
|
||||
public void testWriteSkipOnError() throws Exception {
|
||||
processor.setWriteSkipPolicy(new AlwaysSkipItemSkipPolicy());
|
||||
processor.setItemWriter(new ItemWriter<String>() {
|
||||
@@ -113,19 +117,10 @@ public class FaultTolerantChunkProcessorTests {
|
||||
processor.process(contribution, inputs);
|
||||
fail("Expected Error");
|
||||
}
|
||||
catch (IllegalStateException e) {
|
||||
assertEquals("Expected Error!", e.getCause().getMessage());
|
||||
catch (Error e) {
|
||||
assertEquals("Expected Error!", e.getMessage());
|
||||
}
|
||||
processor.process(contribution, inputs);
|
||||
try {
|
||||
processor.process(contribution, inputs);
|
||||
fail("Expected Error");
|
||||
}
|
||||
catch (IllegalStateException e) {
|
||||
assertEquals("Expected Error!", e.getCause().getMessage());
|
||||
}
|
||||
assertEquals(1, contribution.getSkipCount());
|
||||
assertEquals(1, contribution.getWriteCount());
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -236,6 +236,22 @@ public class FaultTolerantStepFactoryBeanRollbackTests {
|
||||
assertEquals(4, stepExecution.getRollbackCount());
|
||||
}
|
||||
|
||||
/**
|
||||
* Scenario: Exception in writer that should not cause rollback and scan
|
||||
*/
|
||||
@Test
|
||||
public void testWriterDefaultRollbackOnError() throws Exception {
|
||||
writer.setFailures("2", "3");
|
||||
writer.setExceptionType(AssertionError.class);
|
||||
|
||||
Step step = (Step) factory.getObject();
|
||||
|
||||
step.execute(stepExecution);
|
||||
assertEquals(BatchStatus.FAILED, stepExecution.getStatus());
|
||||
assertEquals(0, stepExecution.getSkipCount());
|
||||
assertEquals(1, stepExecution.getRollbackCount());
|
||||
}
|
||||
|
||||
/**
|
||||
* Scenario: Exception in writer that should not cause rollback and scan
|
||||
*/
|
||||
|
||||
@@ -26,7 +26,6 @@ import java.util.ArrayList;
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
|
||||
import org.apache.commons.lang.mutable.MutableInt;
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
import org.springframework.batch.core.BatchStatus;
|
||||
|
||||
Reference in New Issue
Block a user