Skip to content

[12.x] ScheduledTaskFailed not dispatched on scheduled forground task fails #55624

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.

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

Open
wants to merge 7 commits into
base: 12.x
Choose a base branch
from

Conversation

achrafAa
Copy link
Contributor

@achrafAa achrafAa commented May 1, 2025

Fix: Consistent Failure Event Dispatching for Foreground and Background Scheduled Tasks

Description

This PR resolves inconsistencies in Laravel's scheduler related to how task failures are handled and how the ScheduledTaskFailed event is dispatched.

Problem

Two main issues were affecting scheduled task failure behavior:

  1. Foreground task failures (e.g., exceptions or non-zero exit codes) did not always result in ScheduledTaskFailed being dispatched properly.

  2. Background tasks using runInBackground() could cause unexpected exceptions when exit codes were not returned or were null, disrupting the scheduler.

This led to unreliable error monitoring and regressions after PR #55572, which had to be reverted due to these side effects.

Solution

This PR ensures:

  • Foreground task failures now reliably dispatch the ScheduledTaskFailed event when a task throws an exception or exits with a non-zero status code.

  • Background tasks (runInBackground()) are excluded from failure exceptions when exit codes are not explicitly returned, preventing unintended scheduler crashes.

  • The scheduler behaves consistently across both execution modes while preserving Laravel's event-driven error handling model.

  • before this fix :
Scenario ScheduledTaskFinished ScheduledTaskFailed
Command runs and throws an exception within itself ✅ Dispatched ❌ Not dispatched
Command runs and returns non-zero $exitCode ✅ Dispatched ❌ Not dispatched
$event->run($this->laravel); itself throws ❌ Not dispatched ✅ Dispatched
  • after the fix :
Scenario ScheduledTaskFinished ScheduledTaskFailed
Command runs and throws an exception within itself ✅ Dispatched ✅ Dispatched
Command runs and returns non-zero $exitCode ✅ Dispatched ✅ Dispatched
$event->run($this->laravel); itself throws ❌ Not dispatched ✅ Dispatched

So to be clear, ScheduledTaskFinished should always be dispatched unless the exception happens before or outside the actual command execution (e.g. command not found, or failure to start).

Impact

  • Ensures accurate and consistent dispatching of the ScheduledTaskFailed event.

  • Prevents exceptions from being thrown for background tasks with null or missing exit codes.

  • Maintains backward compatibility and avoids introducing regressions.

Related Issues

achrafAa added 4 commits May 1, 2025 23:27
- Throw an exception for scheduled commands that fail in the foreground, providing the exit code in the message.
- Introduce a new test suite to verify the behavior of scheduled commands, ensuring that events are dispatched correctly for failed, successful, and background tasks.
Copy link

github-actions bot commented May 1, 2025

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.

@achrafAa
Copy link
Contributor Author

achrafAa commented May 1, 2025

if there is any other case you want me to test or handle, feel free to drop a comment. thanks

@achrafAa
Copy link
Contributor Author

achrafAa commented May 1, 2025

@erickcomp
@tarkis

@achrafAa achrafAa changed the title [12.x] scheduled task failed not dispatched on scheduled forground task fails [12.x] ScheduledTaskFailed not dispatched on scheduled forground task fails May 1, 2025
@erickcomp
Copy link
Contributor

Now that it's restricted to foreground tasks, I think that no side effects will be encountered.

Thanks for your work, @achrafAa!

@achrafAa
Copy link
Contributor Author

achrafAa commented May 2, 2025

most welcome, always happy to help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scheduled Task Failure Events Not Consistently Dispatched (Including runInBackground Cases)
3 participants