Queue worker does not respect `maxExceptions`
Setting the maxExceptions property on a job has no effect. For instance, the following job will be tried 25 times even though it throws an exception every time:
class ProcessPodcast implements ShouldQueue
{
use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
public $tries = 25;
public $maxExceptions = 3;
public function handle(): void {
throw new Exception('The podcast cannot be processed');
}
}
I believe this is because the base worker class uses the cache to count the number of exceptions (see the markJobAsFailedIfWillExceedMaxExceptions method). The cache is set with $worker->setCache(..) in Laravels WorkCommand, but I don't believe this is done in Laravel Bridge.
@tillkruss you're using this extensively I believe, any idea?
Also even if the cache was set, I believe the user must be using DynamoDB or Redis as a centralized cache (else this is the disk cache), so that might be worth clarifying in the docs, right?
Jup, the queue worker depends on cache to work.
@georgeboot but as @marcusrettig pointed out, it seems the cache is not set?
Do we have 2 problems here:
- the cache instance is not set on the worker
- the documentation doesn't indicate that one must use a centralized cache
or do we have just one?
@mnapoli Thank you for your quick response! I do believe we have both problems. A centralized cache would only be required for jobs that use maxExceptions though.
@marcusrettig got it! Would you be able to test in your case that with ->setCache (and after setting a centralized cache) things work correctly?
Then if you have time for a PR we can merge it. If you don't I could try to work on it but it would take more time because I'm tight on time right now.
@mnapoli I will test it later today or early next week, and if it solves the issue I can open a PR.
@mnapoli I created #122 with the basic fix. I tested it on a single worker (maxConcurrency: 1) using file cache, and it worked. I assume it will also work using a centralized cache, but I will have to verify this later in our staging environment.
Thanks @marcusrettig!
I'm keeping this issue open because we need to document that either Redis or DynamoDB must be used as Laravel cache on this page: https://bref.sh/docs/frameworks/laravel.html#laravel-queues