-
Notifications
You must be signed in to change notification settings - Fork 11.7k
Closed
Labels
Description
- Laravel Version: 8.68.1
- PHP Version: 7.4.21
- Database Driver & Version: N/A
Description:
When you call Event::withoutOverlapping() it sets up an "after" callback that removes the event mutex:
$this->then(function () {
$this->mutex->forget($this);
});If any other before or after callback fails before the mutex is cleared, this callback won't be triggered.
Steps To Reproduce:
$schedule->command('inspire')
->everyMinute()
->withoutOverlapping()
->before(function() {
throw new \Exception('Something unexpected happened.');
});Potential Solutions:
There are three possible solutions that I can think of:
- Run each "before" and "after" callback in a
try/catchto ensure that they all run. This would fix the problem, but also potentially break code that expects previous callbacks to have run. - Add special handling for the "withoutOverlapping" case, maybe by just checking the
$withoutOverlappingvalue afterEvent::run()has executed, and clearing the mutex there. - Introduce a new kind of callback… something like
Event::alwaysBefore()andEvent::alwaysAfter()that is executed separately fromEvent::before()andEvent::after()and is run within atry/catchblock to ensure that they always run no matter what. We could then move the call to$mutex->forget()intoalwaysAfter()to ensure the mutex is cleared regardless of what exceptions were thrown.
I'm happy to PR a solution, but I wanted some feedback before I do.
Additional Context:
This came up because we have a Log::info() call inside a before() callback, and our logging service had downtime causing an exception to be thrown. We've added try/catch statements to our application code and added an exception handler to that logger, but it was a beast to debug. I ended up having to manually find and clear the mutex value in our Redis cache, which was not fun!