WASdev / standards.jsr352.jbatch

Home of 'jbatch', a compatible implementation of the Jakarta Batch specification (and the former Reference Implementation for the JSR 352, Batch Applications for the Java Platform specification).
Other
21 stars 18 forks source link

StepListener.afterStep() should not be called in case of exceptions inside the step #21

Closed mariok closed 9 years ago

mariok commented 9 years ago

Please see also: https://issues.apache.org/jira/browse/BATCHEE-57

Note: This issue is somehow related to https://github.com/WASdev/standards.jsr352.jbatch/issues/20

As we have some integration-tests running on different application-servers, we discovered some differences in the behavior of the jbatch-implementations concerning listeners in case of exceptions:

If an exception occurs during step-processing (Batchlet- as well as Chunk-Steps), then in case of JBeret, the StepListener.afterStep() method is not called, while the JBatch RI included in GlassFish 4 as well as Apache BatchEE do call the method.

This issue has already been discussed in the JBeret issue-tracker:

https://github.com/jberet/jsr352/issues/25

Following the logic of the ChunkListener.afterChunk() method, I think that the afterStep()-method should not be called in case of exceptions. So I think the BatchEE implementation should be adjusted here. Please refer to the JBeret issue linked above for more details.

I think in case of exceptions, something like a StepListener.onError() method would be a good idea (instead of calling afterStep() ). I have opened a discussion on the jbatch public mailing list for enhancing the error-handling with listeners:

https://java.net/projects/jbatch/lists/public/archive/2014-12/message/3

BrentDouglas commented 9 years ago

This is incorrect. Section 9.2.2 of the spec:

A step listener can receive control before and after a step runs, and also if an exception is thrown during step processing.

JBeret should be calling afterStep.

mariok commented 9 years ago

Correct. I guess I was concentrating too much on the JavaDoc of the afterStep() method and did not see the piece of information in the introductory sentence. Thanks for pointing this out. This issue can be closed then, of course. I will open one for JBeret and comment on the corresponding BatchEE issue as well.

Btw, what is the reason, that there is no onError() method as in ChunkListener here? Would be great if you could comment on this on the linked jbatch spec discussion list.

BrentDouglas commented 9 years ago

I had no part in the design of JSR-352 but I think I grok the concepts they were going for pretty well. If you look at all the listeners, the step and job listeners can be thought of as representing a try-finally block while the chunk ones can be thought of as a try-catch block. I would imagine that this is because both step and jobs can have multiple children. Take for example a contrived case where you have a step that runs two parallel chunks, one of which fails and the other succeeds. Who you gonna call?

Any cleanup you need to do based on the outcome of a chunk should really be done in the chunk listener where you get the outcome of the chunk you are working on. If you really need to do some cleanup in the step listener based on if a chunk failed you can always inject the StepContext and test if the BatchStatus == FAILED which indicates that the chunk failed (or, in a parallel chunk, that at least one of the chunks failed).