Skip to content

Commit

Permalink
Extract duplicate code which handles job failing.
Browse files Browse the repository at this point in the history
  • Loading branch information
taylorotwell committed Dec 28, 2016
1 parent 534889e commit 55afe12
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 29 deletions.
48 changes: 48 additions & 0 deletions src/Illuminate/Queue/FailingJob.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

namespace Illuminate\Queue;

use Illuminate\Container\Container;
use Illuminate\Queue\Events\JobFailed;
use Illuminate\Contracts\Events\Dispatcher;

class FailingJob
{
/**
* Delete the given job, call the "failed" method, and raise the failed job event.
*
* @param string $connectionName
* @param \Illuminate\Queue\Jobs\Job $job
* @param \Exception $e
* @return void
*/
public static function handle($connectionName, $job, $e = null)
{
if ($job->isDeleted()) {
return;
}

try {
// If the job has failed, we will delete it, call the "failed" method and then call
// an event indicating the job has failed so it can be logged if needed. This is
// to allow every developer to better keep monitor of their failed queue jobs.
$job->delete();

$job->failed($e);
} finally {
static::events()->fire(new JobFailed(
$connectionName, $job, $e ?: new ManuallyFailedException
));
}
}

/**
* Get the event dispatcher instance.
*
* @return \Illuminate\Contracts\Events\Dispatcher
*/
protected static function events()
{
return Container::getInstance()->make(Dispatcher::class);
}
}
14 changes: 2 additions & 12 deletions src/Illuminate/Queue/InteractsWithQueue.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,8 @@ public function delete()
*/
public function fail($exception = null)
{
if (! $this->job || $this->job->isDeleted()) {
return;
}

try {
$this->job->delete();

$this->job->failed($e);
} finally {
Container::getInstance()->make(Dispatcher::class)->fire(new Events\JobFailed(
$this->job->getConnectionName(), $this->job, $exception ?: new ManuallyFailedException
));
if ($this->job) {
FailingJob::handle($this->job->getConnectionName(), $this->job, $exception);
}
}

Expand Down
17 changes: 1 addition & 16 deletions src/Illuminate/Queue/Worker.php
Original file line number Diff line number Diff line change
Expand Up @@ -324,29 +324,14 @@ protected function markJobAsFailedIfHasExceededMaxAttempts(
/**
* Mark the given job as failed and raise the relevant event.
*
* Note: Any change to this method should also be made to InteractsWithQueue.
*
* @param string $connectionName
* @param \Illuminate\Contracts\Queue\Job $job
* @param \Exception $e
* @return void
*/
protected function failJob($connectionName, $job, $e)
{
if ($job->isDeleted()) {
return;
}

try {
// If the job has failed, we will delete it, call the "failed" method and then call
// an event indicating the job has failed so it can be logged if needed. This is
// to allow every developer to better keep monitor of their failed queue jobs.
$job->delete();

$job->failed($e);
} finally {
$this->raiseFailedJobEvent($connectionName, $job, $e);
}
return FailingJob::handle($connectionName, $job, $e);
}

/**
Expand Down
15 changes: 14 additions & 1 deletion tests/Queue/QueueWorkerTest.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?php

use Illuminate\Container\Container;
use Illuminate\Queue\WorkerOptions;
use Illuminate\Queue\Events\JobFailed;
use Illuminate\Queue\Events\JobProcessed;
Expand All @@ -14,10 +15,20 @@ class QueueWorkerTest extends PHPUnit_Framework_TestCase
public $events;
public $exceptionHandler;

public function __construct()
public function setUp()
{
$this->events = Mockery::spy(Dispatcher::class);
$this->exceptionHandler = Mockery::spy(ExceptionHandler::class);

Container::setInstance($container = new Container);

$container->instance(Dispatcher::class, $this->events);
$container->instance(ExceptionHandler::class, $this->exceptionHandler);
}

public function tearDown()
{
Container::setInstance();
}

public function test_job_can_be_fired()
Expand Down Expand Up @@ -115,6 +126,7 @@ public function test_job_is_failed_if_it_has_already_exceeded_max_attempts()
$job = new WorkerFakeJob(function ($job) {
$job->attempts++;
});

$job->attempts = 2;

$worker = $this->getWorker('default', ['queue' => [$job]]);
Expand Down Expand Up @@ -167,6 +179,7 @@ private function workerDependencies($connectionName = 'default', $jobs = [])
private function workerOptions(array $overrides = [])
{
$options = new WorkerOptions;

foreach ($overrides as $key => $value) {
$options->{$key} = $value;
}
Expand Down

0 comments on commit 55afe12

Please sign in to comment.