Job-based retryAfter() method is called just once thus doesn't allow different values based on number of attempts

[Stalinko]

• Laravel Version: 5.8.35
• PHP Version: 7.2.22 (doesn't really matter)
• Database Driver & Version: MySQL 5.7

Description:

This issue is related to this feature:
laravel/ideas#537
#28265

If you implement a job-class with a custom retryAfter() method then it will be called just once when the job is created and for all further attempts same delay value will be used.

Steps To Reproduce:

Just create a app/Jobs/TestJob.php class with the following contents:

<?php

namespace App\Jobs;

use Illuminate\Bus\Queueable;
use Illuminate\Queue\SerializesModels;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;

class TestJob implements ShouldQueue
{
    use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;

    /**
     * Execute the job.
     *
     * @return void
     * @throws \Exception
     */
    public function handle()
    {
        throw new \Exception('Fail!');
    }

    public function retryAfter()
    {
        return $this->attempts() > 1 ? 60 : 10;
    }
}

Set database as the queue driver and start the queue.
Expected: first time the delay should be 10 seconds, in all further tries - 60 seconds.
Actual result: it always applies 10 seconds delay.

[driesvints]

Hey there,

Unfortunately we don't support this version anymore. Please check out our support policy on which versions we are currently supporting. Can you please try to upgrade to the latest version and see if your problem persists? We'll help you out and re-open this issue if so.

Thanks!

[bdenizar]

Hello,

I stucked on the same issue than you, i wanted to implement backoff exponential on my retry attempt.

Here is the solution i found working for redis queue (based on the same job as you):

<?php

namespace App\Jobs\Listeners;

use App\BackOff;
use Illuminate\Queue\Events\JobExceptionOccurred;

class JobEventSubscriber
{
    /**
     * Handle the event.
     *
     * @param  JobExceptionOccurred  $event
     * @return void
     */
    public function handleJobException(JobExceptionOccurred $event)
    {
        $job = $event->job;

        if ($job->isDeleted() || $job->isReleased() || $job->hasFailed()) {
            // @see https://github.com/laravel/framework/blob/6.x/src/Illuminate/Queue/Worker.php#L377
            return;
        }

        // Convert ms to s because releasing job is in s
        $delay = (int) (BackOff::decorrelatedJitter(1000, $job->attempts(), 60000) / 1000);
        $job->release($delay);
    }

    /**
     * Register the listeners for the subscriber.
     *
     * @param  \Illuminate\Events\Dispatcher  $events
     */
    public function subscribe($events)
    {
        $me = static::class;

        $events->listen(
            JobExceptionOccurred::class,
            "{$me}@handleJobException"
        );
    }
}

<?php

namespace App\Jobs;

use Exception;
use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;

class TestJob implements ShouldQueue
{
    use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;

    /**
     * The number of times the job may be attempted.
     *
     * @var int
     */
    public $tries = 0;

    /**
     * Create a new job instance.
     *
     * @return void
     */
    public function __construct()
    {
        //
    }

    /**
     * Execute the job.
     *
     * @return void
     */
    public function handle()
    {
        throw new Exception('Fail!');
    }

    /**
     * Determine the time at which the job should timeout.
     *
     * @return \DateTime
     */
    public function retryUntil()
    {
        return now()->addSeconds(300);
    }
}

Register the listener on EventServiceProvider:

    /**
     * The subscriber classes to register.
     *
     * @var array
     */
    protected $subscribe = [
        JobEventSubscriber::class
    ];

Test: php artisan queue:work --tries=3 --delay=3
Result:

Hope it will help you.

[adm-bome]

Issue still persists. (latest version)

One would expect when defining this method, the outcome of this method can be flexible.

When the worker only call's this method once (in the beginning of building the queued message), the delay is almost as static as setting delay attribute on the command or with the delay method on the job.

When the worker call's this method every time a jobs fails ( note: not marked as failed) or before the next try, then we could influence the delay-time (when the job will be available for retry).

This will result in a sort off BackoffStrategy when needed.

[adm-bome]

After some research..... The retryAfter method does what it's supposed to do.

The workers aren't aware of delay's, and do not alter the payload. (as it should be)

As mentioned in my previous post the retryAfter method should be flexible. This is true as long the payload is not created. When publishing the Job the payload should be static.

When one feels the need to alter the payload, in essence the original Job was already broken.

As @bdenizar pointed out, there is an event dispatched when a job is failed. Source
A solution for a BackoffStrategy could be hooking into this event.

This solution is based on @bdenizar comment. The difference is that Backoff is defined in the Jobs.
When a Job doesn't define a method retryDelay and an specific delay ($this->delay) or retryAfter(). The Worker will use the defaults.

When the method retryDelay returns an falsified value, the job is not released. So the worker can do the release as ususal. Source

This will result in an situation where different Jobs can live inside the same Queue with a different behavior (note: backoff / Delay).

// app/Providers/EventServiceProvider.php
...

 protected $listen = [
        JobExceptionOccurred::class => [
            BackoffFailedJob::class,
        ],
    ];

...

// app/Listeners/BackoffFailedJob.php
<?php

namespace App\Listeners;

use DateTimeInterface;
use Illuminate\Contracts\Queue\Job;
use Illuminate\Queue\Events\JobExceptionOccurred;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Support\InteractsWithTime;


class BackoffFailedJob
{
    use InteractsWithTime;

    /**
     * Create the event listener.
     *
     * @return void
     */
    public function __construct()
    {
        //
    }

    /**
     * Handle the event.
     *
     * @param  JobExceptionOccurred  $event
     * @return void
     */
    public function handle(JobExceptionOccurred $event)
    {
        $job = $event->job;

        if ($job->isDeleted() || $job->isReleased() || $job->hasFailed()) {
            return;
        }

        if (! $backoffDelay = $this->getBackoffDelay($job)) {
            return;
        }

        $job->release($backoffDelay);
    }

    /**
     * Get the backoff delay in seconds from the job's command
     *
     * @param $command
     * @return int|void
     */
    private function getBackoffDelay(Job $job)
    {
        if (! $command = $this->getCommand($job)) {
            return;
        }

        if (! method_exists($command, 'retryDelay')) {
            return;
        }

        if (! $delay = $command->retryDelay()) {
            return;
        };

        return $delay instanceof DateTimeInterface ? $this->secondsUntil($delay) : $delay;
    }

    /**
     * Get the Command within the current job
     *
     * @param Job $job
     * @return mixed
     */
    private function getCommand(Job $job)
    {
        $command = unserialize($job->payload()['data']['command']);

        if (in_array(InteractsWithQueue::class, class_uses_recursive($command))) {
            $command->setJob($job);
        }

        return $command;
    }
}

// app/Jobs/TestJob.php

<?php

namespace App\Jobs;

use App\Jobs\Middleware\DelayJobMiddleware;
use App\Temperature;
use DateTimeInterface;
use Exception;
use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;
use Illuminate\Support\Str;

class TestJob implements ShouldQueue
{
    use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;

   /**
     * Create a new job instance.
     *
     * @param Temperature $temperature
     */
    public function __construct()
    {
          $this->delay(10);   // Or define a method retryAfter()
    }

   /**
     * Execute the job.
     *
     * @return void
     * @throws Exception
     */
    public function handle()
    {
        throw new Exception(sprintf('Job %s failed.', Str::kebab(class_basename(self::class))));
    }

    /**
     * When Failed
     *
     * @param Exception|null $exception
     */
    public function failed(?Exception $exception = null)
    {
        //
    }

   /**
     * Can returns an delay
     *
     * Used when the App\Listeners\BackoffFailedJob listener is registered.
     *
     * @return DateTimeInterface|int|void
     */
    public function retryDelay()
    {
        if (! $delay = $this->delay ?? $this->jobDelay() ?? null) {
            return;
        }

        return $this->calculateDelay($delay);
    }

    /**
     * Get the delay of de Job
     *
     * @return int|null
     */
    private function jobDelay()
    {
        return $this->job ? $this->job->payload()['delay'] : null;
    }

    /**
     * @param int $delay
     * @return int
     */
    private function calculateDelay(int $delay = 0): int
    {
        return $delay * $this->attempts();
    }

}