Batched jobs and then callback behavour is weird

[morloderex]

• Laravel Version: 8.51.0
• PHP Version: 8.0.8
• Database Driver & Version: mysql 8.0.22

Description:

If an exception is thrown during the execution in the then mehod on a batched job. The last job executed gets marked as failed and therefore the batch has been considered as a failure in the context of without allowFailures(). Which seems a bit weird to me, because that's actually not what has happened.

You get some weird behavour in this situation here is my findings during debugging:

1. The last executed job get's marked as failed when really it has not.
2. When you retry the failed batch due to the false failure the then method is not triggered the second time sense the batch's pendingJobs table reaches a value bellow 0.

Steps To Reproduce:

Bus::batch([
    function () {
      echo 'job 1'; 
},
    function () {
      echo job 2'; 
},
    function () {
      echo 'job 3';
}
])
->then(function (Batch $batch) {
    throw new Exception('then method failed')
})->dispatch();

[taylorotwell]

Hmm, we should probably explore this @themsaid

[themsaid]

This is expected yes with the current way things are configured. The then callbacks are invoked right after the last job in the batch succeeds and before deleting it from the queue. If an exception is thrown, it's considered to be within the job failure.

We could technically swallow the exception inside try...catch and report it using report() and just finish the batch. But that way the batch will finish and there won't be any way to rerun the logic in the then callbacks. What do you think @taylorotwell ?

[morloderex]

@themsaid I understand your point of view that if we just swallow the exception we cannot re execute the then method.
That being said you currently also aren't able to rerun the then method with the current behavour unless you manually increase the pendingjobs column from -1 to +1 either.

Sense the job first succeeded and then the then method executed which threw the exception which also failed the job which in turn also decreased the pendingjobs column down to -1.

We should probably look at this if we decide to keep the current behavour.

[taylorotwell]

I personally agree that we should probably catch the exception, report it, and let the batch finish. Even though you can't re-run the then callback it seems like a better situation than what we have now.