diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecution.java b/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecution.java index 65cf5b9a9..d5e0527ae 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecution.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/StepExecution.java @@ -57,7 +57,7 @@ public class StepExecution extends Entity { private volatile ExecutionContext executionContext = new ExecutionContext(); - private volatile ExitStatus exitStatus = ExitStatus.UNKNOWN; + private volatile ExitStatus exitStatus = ExitStatus.CONTINUABLE; private volatile boolean terminateOnly; diff --git a/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java b/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java index fbf81ecff..8344c3a1c 100644 --- a/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java +++ b/spring-batch-core/src/main/java/org/springframework/batch/core/step/AbstractStep.java @@ -150,7 +150,8 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw * resource initialization ({@link #open(ExecutionContext)}), execution * logic ({@link #doExecute(StepExecution)}) and resource closing ({@link #close(ExecutionContext)}). */ - public final void execute(StepExecution stepExecution) throws JobInterruptedException, UnexpectedJobExecutionException { + public final void execute(StepExecution stepExecution) throws JobInterruptedException, + UnexpectedJobExecutionException { stepExecution.setStartTime(new Date()); stepExecution.setStatus(BatchStatus.STARTED); @@ -176,13 +177,13 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw } catch (Exception e) { commitException = e; - stepExecution.setStatus(BatchStatus.UNKNOWN); + exitStatus = exitStatus.and(ExitStatus.UNKNOWN); } } catch (Throwable e) { - logger.error("Encountered an error executing the step: "+e.getClass()+": "+e.getMessage()); + logger.error("Encountered an error executing the step: " + e.getClass() + ": " + e.getMessage()); stepExecution.setStatus(determineBatchStatus(e)); exitStatus = getDefaultExitStatusForFailure(e); @@ -191,7 +192,7 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw getJobRepository().saveOrUpdateExecutionContext(stepExecution); } catch (Exception ex) { - logger.error("Encountered an error on listener close.", ex); + logger.error("Encountered an error on listener error callback.", ex); } rethrow(e); } @@ -204,18 +205,24 @@ public abstract class AbstractStep implements Step, InitializingBean, BeanNameAw getJobRepository().saveOrUpdate(stepExecution); } catch (Exception e) { - commitException = e; + if (commitException == null) { + commitException = e; + } + else { + logger.error("Exception while updating step execution after commit exception", e); + } } try { close(stepExecution.getExecutionContext()); } catch (Exception e) { - logger.error("Exception while closing step's resources", e); - throw new UnexpectedJobExecutionException("Exception while closing step's resources", e); + logger.error("Exception while closing step execution resources", e); + throw new UnexpectedJobExecutionException("Exception while closing step resources", e); } if (commitException != null) { + stepExecution.setStatus(BatchStatus.UNKNOWN); logger.error("Encountered an error saving batch meta data." + "This job is now in an unknown state and should not be restarted.", commitException); throw new UnexpectedJobExecutionException("Encountered an error saving batch meta data.", diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/StepExecutionTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/StepExecutionTests.java index e9b18a377..bed4665e8 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/StepExecutionTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/StepExecutionTests.java @@ -88,7 +88,7 @@ public class StepExecutionTests extends TestCase { * {@link org.springframework.batch.core.JobExecution#getExitStatus()}. */ public void testGetExitCode() { - assertEquals(ExitStatus.UNKNOWN, execution.getExitStatus()); + assertEquals(ExitStatus.CONTINUABLE, execution.getExitStatus()); execution.setExitStatus(ExitStatus.FINISHED); assertEquals(ExitStatus.FINISHED, execution.getExitStatus()); } diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/AbstractStepTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/AbstractStepTests.java index 78445d2e9..2e6c6814a 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/AbstractStepTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/AbstractStepTests.java @@ -170,11 +170,45 @@ public class AbstractStepTests extends TestCase { assertEquals(7, events.size()); assertEquals(ExitStatus.FAILED.getExitCode(), execution.getExitStatus().getExitCode()); - + assertTrue("Execution context modifications made by listener should be persisted", repository.saved .containsKey("onErrorInStep")); } + /** + * Exception during business processing. + */ + public void testFailureInSavingExecutionContext() throws Exception { + tested = new EventTrackingStep() { + protected ExitStatus doExecute(StepExecution stepExecution) throws Exception { + super.doExecute(stepExecution); + return ExitStatus.FINISHED; + } + }; + repository = new JobRepositoryStub() { + public void saveOrUpdateExecutionContext(StepExecution stepExecution) { + throw new RuntimeException("Bad context!"); + } + }; + tested.setJobRepository(repository); + + try { + tested.execute(execution); + fail(); + } + catch (RuntimeException expected) { + assertEquals("Bad context!", expected.getCause().getMessage()); + } + + int i = 0; + assertEquals("open", events.get(i++)); + assertEquals("doExecute", events.get(i++)); + assertEquals("close", events.get(i++)); + assertEquals(3, events.size()); + + assertEquals(ExitStatus.UNKNOWN.getExitCode(), execution.getExitStatus().getExitCode()); + } + /** * JobRepository is a required property. */ diff --git a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ItemOrientedStepTests.java b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ItemOrientedStepTests.java index e48e7dc67..5b315db84 100644 --- a/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ItemOrientedStepTests.java +++ b/spring-batch-core/src/test/java/org/springframework/batch/core/step/item/ItemOrientedStepTests.java @@ -76,7 +76,7 @@ public class ItemOrientedStepTests extends TestCase { private ItemOrientedStep itemOrientedStep; private Job job; - + private JobInstance jobInstance; private ResourcelessTransactionManager transactionManager; @@ -314,7 +314,6 @@ public class ItemOrientedStepTests extends TestCase { assertEquals(BatchStatus.UNKNOWN, stepExecution.getStatus()); } - /* * Test that a job that is being restarted, but has saveExecutionAttributes * set to false, doesn't have restore or getExecutionAttributes called on @@ -539,7 +538,8 @@ public class ItemOrientedStepTests extends TestCase { catch (JobInterruptedException ex) { assertEquals(BatchStatus.STOPPED, stepExecution.getStatus()); String msg = stepExecution.getExitStatus().getExitDescription(); - assertTrue("Message does not contain 'JobInterruptedException': " + msg, contains(msg, "JobInterruptedException")); + assertTrue("Message does not contain 'JobInterruptedException': " + msg, contains(msg, + "JobInterruptedException")); } } @@ -664,14 +664,12 @@ public class ItemOrientedStepTests extends TestCase { public void testStatusForFinalUpdateFailedException() throws Exception { - itemOrientedStep.setJobRepository(new JobRepositorySupport() { - public void saveOrUpdate(StepExecution stepExecution) { - if (stepExecution.getEndTime() != null) { - throw new RuntimeException("Bar"); - } - super.saveOrUpdate(stepExecution); + itemOrientedStep.setJobRepository(new JobRepositorySupport()); + itemOrientedStep.setStreams(new ItemStream[] { new ItemStreamSupport() { + public void close(ExecutionContext executionContext) throws ItemStreamException { + throw new RuntimeException("Bar"); } - }); + } }); JobExecution jobExecutionContext = new JobExecution(jobInstance); StepExecution stepExecution = new StepExecution(itemOrientedStep.getName(), jobExecutionContext); @@ -681,12 +679,13 @@ public class ItemOrientedStepTests extends TestCase { fail("Expected RuntimeException"); } catch (RuntimeException ex) { - // The job actually completeed, but teh streams couldn't be closed. + // The job actually completed, but the streams couldn't be closed. assertEquals(BatchStatus.COMPLETED, stepExecution.getStatus()); String msg = stepExecution.getExitStatus().getExitDescription(); assertEquals("", msg); msg = ex.getMessage(); - assertTrue("Message does not contain 'saving batch meta data': " + msg, contains(msg, "saving batch meta data")); + assertTrue("Message does not contain 'closing step': " + msg, contains(msg, + "closing step")); // The original rollback was caused by this one: assertEquals("Bar", ex.getCause().getMessage()); } @@ -754,7 +753,7 @@ public class ItemOrientedStepTests extends TestCase { assertTrue(stepExecution.getExecutionContext().getString("spam").equals("bucket")); } } - + public void testStepToCompletion() throws Exception { RepeatTemplate template = new RepeatTemplate(); @@ -769,7 +768,7 @@ public class ItemOrientedStepTests extends TestCase { itemOrientedStep.execute(stepExecution); assertEquals(3, processed.size()); assertEquals(3, stepExecution.getItemCount().intValue()); - } + } /** * Exception in {@link StepExecutionListener#afterStep(StepExecution)}