framework icon indicating copy to clipboard operation
framework copied to clipboard

[12.x] Introduce `FailOnException` job middleware

Open cosmastech opened this issue 5 months ago • 6 comments

Why

I want a job to not retry when a particular exception is thrown. For the life of me, outside of calling $this->fail() inside the job itself, I do not see a clean way of doing this. Hence this middleware.

Example

class MyJob implements ShouldQueue
{
    use InteractsWithQueue;
    
    public function __construct(private array $snsPayload) {}

    public function handle(GetUserFromExternalApiAction $getUserAction): void
    {
        // potentially throws a UserDoesNotExistException
        $user = $getUserAction->handle($this->snsPayload['user_id']);

        // do some other stuff with the user based on the SNS event.
    }

    public function middleware(): array
    {
        return [new FailOnException(fn (\Throwable $e) => ! $e instanceof UserDoesNotExistException)];
        // or more succinctly
        return [new FailOnException([UserDoesNotExistException::class])];
    }
}

cosmastech avatar Jun 13 '25 16:06 cosmastech

Just my 2 cents: I would love if we can use RetryWhen instead. when is used everywhere else and sounds more like a sentence. We then could also implement the RetryUnless version.

henzeb avatar Jun 14 '25 12:06 henzeb

Just my 2 cents: I would love if we can use RetryWhen instead. when is used everywhere else and sounds more like a sentence. We then could also implement the RetryUnless version.

Been kind of going back and forth on the name this morning. It seems like a better name would be that this is a circuit breaker. As it's written now, it may seem that it's extending the number of tries (on the job or queue level).

cosmastech avatar Jun 14 '25 12:06 cosmastech

I was also thinking about the name, maybe FailIf would better convey the outcome?

Of course, in that case, the condition should be reversed to:

if (call_user_func($this->failIf, $e, $job) === true) {
    $job->fail($e);
}

But perhaps that communicates better the intent, while still achieving the desired outcome (allow to short circuit retrials on an specfic set of exceptions).

rodrigopedra avatar Jun 14 '25 19:06 rodrigopedra

I was also thinking about the name, maybe FailIf would better convey the outcome?

Of course, in that case, the condition should be reversed to:

if (call_user_func($this->failIf, $e, $job) === true) {
    $job->fail($e);
}

But perhaps that communicates better the intent, while still achieving the desired outcome (allow to short circuit retrials on an specfic set of exceptions).

FailIf may be a little bit wonky too, since technically middleware runs before the job is reached, so it may seem like it would prevent the job from running.

We'll see what the maintainer says. In my head, I'm thinking something like ShortCircuitRetriesWhen. Or even just making it an optional method on the job itself. (We still have all those naming problems if it were moved, but just wanted to float that thought out there.)

cosmastech avatar Jun 16 '25 16:06 cosmastech

I would suggest renaming the class to reflect the main use case we have in mind... something like FailIfExceptions(fn|exceptions) or something like that.

taylorotwell avatar Jun 16 '25 17:06 taylorotwell

I would suggest renaming the class to reflect the main use case we have in mind... something like FailIfExceptions(fn|exceptions) or something like that.

Went with FailOnException. Modified the constructor to match your suggestion as well @taylorotwell.

cosmastech avatar Jun 16 '25 23:06 cosmastech

Thanks! Could you send me a docs PR for this? @cosmastech

taylorotwell avatar Jun 17 '25 16:06 taylorotwell