Skip to content

[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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: 12.x
Choose a base branch
from
+57 −35
Draft

[12.x] Batch Job Failure Callbacks Support #55916

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 34 additions & 29 deletions src/Illuminate/Bus/Batch.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,32 +242,33 @@ public function recordSuccessfulJob(string $jobId)
$counts = $this->decrementPendingJobs($jobId);

if ($this->hasProgressCallbacks()) {
$batch = $this->fresh();

(new Collection($this->options['progress']))->each(function ($handler) use ($batch) {
$this->invokeHandlerCallback($handler, $batch);
});
$this->invokeCallbacks('progress');
}

if ($counts->pendingJobs === 0) {
$this->repository->markAsFinished($this->id);
}

if ($counts->pendingJobs === 0 && $this->hasThenCallbacks()) {
$batch = $this->fresh();

(new Collection($this->options['then']))->each(function ($handler) use ($batch) {
$this->invokeHandlerCallback($handler, $batch);
});
$this->invokeCallbacks('then');
}

if ($counts->allJobsHaveRanExactlyOnce() && $this->hasFinallyCallbacks()) {
$batch = $this->fresh();
$this->invokeCallbacks('finally');
}
}

(new Collection($this->options['finally']))->each(function ($handler) use ($batch) {
$this->invokeHandlerCallback($handler, $batch);
/**
* Invoke the callbacks of the given type.
*/
public function invokeCallbacks(string $type, ?\Throwable $e = null): void
{
$batch = $this->fresh();

(new Collection($this->options[$type]))
->each(function ($handler) use ($batch, $e) {
$this->invokeHandlerCallback($handler, $batch, $e);
});
}
}

/**
Expand Down Expand Up @@ -346,28 +347,22 @@ public function recordFailedJob(string $jobId, $e)
$this->cancel();
}

if ($this->hasProgressCallbacks() && $this->allowsFailures()) {
$batch = $this->fresh();
if ($this->allowsFailures()) {
if ($this->hasProgressCallbacks()) {
$this->invokeCallbacks('progress', $e);
}

(new Collection($this->options['progress']))->each(function ($handler) use ($batch, $e) {
$this->invokeHandlerCallback($handler, $batch, $e);
});
if ($this->hasFailureCallbacks()) {
$this->invokeCallbacks('failure', $e);
}
}

if ($counts->failedJobs === 1 && $this->hasCatchCallbacks()) {
$batch = $this->fresh();

(new Collection($this->options['catch']))->each(function ($handler) use ($batch, $e) {
$this->invokeHandlerCallback($handler, $batch, $e);
});
$this->invokeCallbacks('catch', $e);
}

if ($counts->allJobsHaveRanExactlyOnce() && $this->hasFinallyCallbacks()) {
$batch = $this->fresh();

(new Collection($this->options['finally']))->each(function ($handler) use ($batch, $e) {
$this->invokeHandlerCallback($handler, $batch, $e);
});
$this->invokeCallbacks('finally');
}
}

Expand Down Expand Up @@ -402,6 +397,16 @@ public function hasFinallyCallbacks()
return isset($this->options['finally']) && ! empty($this->options['finally']);
}

/**
* Determine if the batch has "failure" callbacks.
*
* @return bool
*/
public function hasFailureCallbacks()
{
return isset($this->options['failure']) && ! empty($this->options['failure']);
}

/**
* Cancel the batch.
*
Expand Down
29 changes: 23 additions & 6 deletions src/Illuminate/Bus/PendingBatch.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,14 +237,23 @@ public function finallyCallbacks()
}

/**
* Indicate that the batch should not be cancelled when a job within the batch fails.
*
* @param bool $allowFailures
* @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
Comment on lines -243 to +244
Copy link
Contributor

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

Suggested change
* @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

Copy link
Contributor

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

Suggested change
* @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

Copy link
Contributor

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

Suggested change
* @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

{
$this->options['allowFailures'] = $allowFailures;
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;
Comment on lines +246 to +256
Copy link
Contributor

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;
}
Expand All @@ -259,6 +268,14 @@ public function allowsFailures()
return Arr::get($this->options, 'allowFailures', false) === true;
}

/**
* Get the "failure" callbacks that have been registered with the pending batch.
*/
public function failureCallbacks(): array
Comment on lines +273 to +274
Copy link
Contributor

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

Suggested change
*/
public function failureCallbacks(): array
*
* @return bool|callable|Closure|array<array-key, callable|Closure>
*/
public function failureCallbacks(): array

{
return $this->options['failure'] ?? [];
}

/**
* Set the name for the batch.
*
Expand Down