-
Notifications
You must be signed in to change notification settings - Fork 11.5k
[12.x] Batch Job Failure Callbacks Support #55916
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 12.x
Are you sure you want to change the base?
Conversation
a7f2f49
to
2f3c496
Compare
Hey @coderabbi - is it possible to remove the formatting changes... just makes the diff easier for me to review quickly. |
On it! |
9c2e8a9
to
b7ba1e1
Compare
b7ba1e1
to
0791d2e
Compare
Done! |
@coderabbi so is this similar to |
Exactly. As I noted in the description, probably more useful when paired with a
|
It's like |
@taylorotwell would love to add tests to this if there's a general inclination towards approval. |
@yitzwillroth yeah I think fine to add tests here. |
if (! is_bool($param)) { | ||
$param = Arr::wrap($param); | ||
|
||
foreach ($param as $callback) { | ||
$this->options['failure'][] = $callback instanceof Closure | ||
? new SerializableClosure($callback) | ||
: $callback; | ||
} | ||
} | ||
|
||
$this->options['allowFailures'] = (bool) $param; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, if $param
isn't a bool
, we cast it to array
and cast that to bool
? Doesn't sound all that logical to me
* @return $this | ||
* Indicate that the batch should not be cancelled when a job within the batch fails; | ||
* optionally add callbacks to be executed upon each job failure (this may be useful for | ||
* running other jobs when a job fails). | ||
*/ | ||
public function allowFailures($allowFailures = true) | ||
public function allowFailures(bool|callable|Closure|array $param = true): self |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can narrow the array
type further down
* @return $this | |
* Indicate that the batch should not be cancelled when a job within the batch fails; | |
* optionally add callbacks to be executed upon each job failure (this may be useful for | |
* running other jobs when a job fails). | |
*/ | |
public function allowFailures($allowFailures = true) | |
public function allowFailures(bool|callable|Closure|array $param = true): self | |
* Indicate that the batch should not be cancelled when a job within the batch fails; | |
* optionally add callbacks to be executed upon each job failure (this may be useful for | |
* running other jobs when a job fails). | |
* | |
* @param bool|callable|Closure|array<array-key, callable|Closure> $param | |
*/ | |
public function allowFailures(bool|callable|Closure|array $param = true): self |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with the callable
and the Closure
* @return $this | |
* Indicate that the batch should not be cancelled when a job within the batch fails; | |
* optionally add callbacks to be executed upon each job failure (this may be useful for | |
* running other jobs when a job fails). | |
*/ | |
public function allowFailures($allowFailures = true) | |
public function allowFailures(bool|callable|Closure|array $param = true): self | |
* Indicate that the batch should not be cancelled when a job within the batch fails; | |
* optionally add callbacks to be executed upon each job failure (this may be useful for | |
* running other jobs when a job fails). | |
* | |
* @param bool|(callable(\Illuminate\Bus\Batch, \Throwable|null): mixed)|Closure(\Illuminate\Bus\Batch, \Throwable|null): mixed)|array<array-key, callable(\Illuminate\Bus\Batch, \Throwable|null): mixed)|Closure(\Illuminate\Bus\Batch, \Throwable|null): mixed)> $param | |
*/ | |
public function allowFailures(bool|callable|Closure|array $param = true): self |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might make sense to DRY this out a bit
* @return $this | |
* Indicate that the batch should not be cancelled when a job within the batch fails; | |
* optionally add callbacks to be executed upon each job failure (this may be useful for | |
* running other jobs when a job fails). | |
*/ | |
public function allowFailures($allowFailures = true) | |
public function allowFailures(bool|callable|Closure|array $param = true): self | |
* Indicate that the batch should not be cancelled when a job within the batch fails; | |
* optionally add callbacks to be executed upon each job failure (this may be useful for | |
* running other jobs when a job fails). | |
* | |
* @template TParam of (callable(\Illuminate\Bus\Batch, \Throwable|null): mixed)|Closure(\Illuminate\Bus\Batch, \Throwable|null): mixed) | |
* | |
* @param bool|TParam|array<array-key, TParam> $param | |
*/ | |
public function allowFailures(bool|callable|Closure|array $param = true): self |
*/ | ||
public function failureCallbacks(): array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can further type this array
*/ | |
public function failureCallbacks(): array | |
* | |
* @return bool|callable|Closure|array<array-key, callable|Closure> | |
*/ | |
public function failureCallbacks(): array |
Would it make sense to also have a mechanism to only invoke on errors when the batch is completed? |
Functional Enhancement
The functional change is in
PendingBatch::allowFailures()
, which now accepts:This allows the registration of callbacks that will be executed when a job fails, providing more granular control over failure handling in batch processing. The
Batch
class has been updated with corresponding methods to handle these new failure callbacks.Of course,
$batch->allowFailures()
and$batch->allowFailures($boolean)
still work as before.Additional Changes
invokeCallbacks
method in the Batch classNotes
$batch->failure($callback)
instead of dual-purposing$batch->allowFailures()
, but went with the latter since the former would require either a call toallowFailures()
anyway, setting this option implicitly in thefailure()
implementation, or reworking the logic ofallowsFailures()
to also return true when failure callbacks are present. I'm open to reworking this in any of those directions.JobContext
object that would be passed to the callback providing more job specific detail, making this potentially more useful.💁🏼♂️ The author is available for hire -- inquire at [email protected].