fhteam/laravel-amqp

If a job throws an exception, the timeout value is null

milsanore opened this issue · 2 comments

My env:

  • Laravel Version: 5.4.36
  • PHP Version: PHP 7.1.12-3+ubuntu14.04.1+deb.sury.org+1
  • Database Driver & Version: jenssegers/laravel-mongodb v3.2
  • Queue driver: (RabbitMQ) fhteam/laravel-amqp

Hi, as always thank you for the brilliant project, it's the backbone of my system.

I have a long-running job (an hour), so I explicitly define the $timeout = 3600; member variable in my job class.

If a job is SIGKILL'ed, the connection to rabbitmq will be severed, and rabbitmq will re-enqueue the message correctly.

However, if I throw an Exception inside my job's handle() function, the message that is re-enqueued has its timeout variable set to null.

I.e. the re-enqueued message looks like the following:

after SIGKILL:

{
    "data": { ... },
    "displayName": "myLvlProject\\Jobs\\MyJob",
    "job": "Illuminate\\Queue\\CallQueuedHandler@call",
    "maxTries": null,
    "timeout": 3600
}

after Exception:

{
    "data": { ... },
    "displayName": "myLvlProject\\Jobs\\MyJob",
    "job": "Illuminate\\Queue\\CallQueuedHandler@call",
    "maxTries": null,
    "timeout": null
}

The problem for me is this: my hour-long job sometimes experiences a network exception.
After this the job is re-enqueued with no timeout value. Note that the default timeout value in Laravel is one minute.
So after my job restarts, it times out after one minute. Repeatedly.
But note, that when a job experiences a timeout, an exception is not throw, rather the process is SIGKILL'ed, and this does not increment the failed count (I have raised a bug for this laravel/framework#22694), therefore my job loops forever.

Steps To Reproduce:

  • Create a job class
  • Explicitly set the timeout value to 3600
  • Throw an exception in the handle() function
  • Enqueue an instance of the job (do not run the queue worker yet)
  • Inspect the JSON payload of the message, note the timeout: 3600 field
  • Dequeue the job (i.e. run the queue worker), the exception will throw, and the job will be re-enqueued
  • Inspect the JSON payload of the message, note the timeout: null field

Thanks,

It is not correct, /vendor/laravel/framework/src/Illuminate/Queue/Worker.php

protected function timeoutForJob($job, WorkerOptions $options)
{
    return $job && ! is_null($job->timeout()) ? $job->timeout() : $options->timeout;
}

It means if value is null it will use timeout from queue worker option ie

php artisan queue:work --timeout=3600

This was fixed in the following pull-request. Closing.