8000 Add deleteWhen for throttle exceptions job middleware by moshe-autoleadstar · Pull Request #55718 · laravel/framework · GitHub
[go: up one dir, main page]
Skip to content

Add deleteWhen for throttle exceptions job middleware #55718

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
May 13, 2025
Merged

Add deleteWhen for throttle exceptions job middleware #55718

merged 7 commits into from
May 13, 2025
+92 −0

Conversation

moshe-autoleadstar
Copy link
Contributor

What

Adds ->deleteWhen(..) to ThrottlesExceptions job middleware (and of course ThrottlesExceptionsWithRedis)

Examples

use Illuminate\Queue\Middleware\ThrottlesExceptions;
 
public function middleware(): array
{
    return [
        new ThrottlesExceptions(10, 5)
            ->deleteWhen(ExceptionThatIsOk::class),
    ];
}
use Illuminate\Queue\Middleware\ThrottlesExceptions;
 
public function middleware(): array
{
    return [
        new ThrottlesExceptions(10, 5)
            ->deleteWhen(fn (\Throwable $throwable) => str($throwable)->contains('its all ok'))
            ->deleteWhen(fn (\Throwable $throwable) => $throwable instanceof \Illuminate\Database\QueryException
                && str($throwable->getMessage())->contains('a foreign key constraint fails')
            ),
    ];
}

Details

When using ThrottlesExceptions job middleware, we can pass a callback for ->when(..) the job should not be released back and instead throw an exception. But for most use cases, you will likely be retrying this job and therefore the job will be attempted again. The new ->deleteWhen(..) functionality allows you to exit the circuit of either throw exception or release back onto the queue, and instead to just delete the job. In this way, the job will just be deleted when this exception occurs. (Of course, you can still choose to report the exception just like before.)

@github-actions GitHub Actions
Copy link

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

@moshe-autoleadstar moshe-autoleadstar mentioned this pull request May 12, 2025
Closed
@moshe-autoleadstar moshe-autoleadstar marked this pull request as ready for review May 12, 2025 15:04
@shaedrich
Copy link
Contributor

Not sure if "delete*" is the most intuitive terminology here 🤔

@moshe-autoleadstar
Copy link
Contributor Author

Not sure if "delete*" is the most intuitive terminology here 🤔

i was also doing a lot of debating about the naming.. i went this way, because that's what we're doing, we're deleting the job. we are in the context of "throttles exceptions" middleware. so maybe ->deleteJobWhen(..) would be better.. just sounded a little overkill..
im open to any other naming here..

@shaedrich
Copy link
Contributor
shaedrich commented May 13, 2025 •
edited
Loading

"Delete" always makes me thing of database or filesystem action. Even though, that what you are doing with the job, it doesn't entirely describe a distinction to what would happen otherwise imo.

Maybe, the naming of when() already is not ideal here. It probably should be something along the lines of releaseUnless() to make it clear what happens there. Assuming, that was the name, how would you have named your new method then?

@moshe-autoleadstar
Copy link
Contributor Author

how about ->skipWhen(..)?
The new "Skip" middleware is actually in the documentation right after the documentation for ThrottlesExceptions. "Skip" basically is delete job.
This terminology also works really well with this middleware itself because we want to "skip" this middleware standard way of working when this callback occurs.

@shaedrich
Copy link
Contributor

Yeah, I guess, this would be more intuitive for me 👍🏻

@taylorotwell taylorotwell merged commit e72d741 into laravel:12.x May 13, 2025
58 checks passed
@milwad-dev milwad-dev mentioned this pull request May 14, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@moshe-autoleadstar @shaedrich @taylorotwell
0