Should ScheduledTaskFinished Be Dispatched When the Task Fails? #55723

achrafAa · 2025-05-13T07:55:21Z

achrafAa
May 13, 2025

The Question

Should the ScheduledTaskFinished event be dispatched if the scheduled task fails?

Context

As it stands, there's a debate about whether it's correct to dispatch the ScheduledTaskFinished event when a task throws an exception or returns a non-zero exit code.

In my recent PR #55624, I tried to improve the current behavior while avoiding breaking changes. Here's a summary of the current vs proposed behavior:

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

Options Considered

1️⃣ First Proposition

✅ Always dispatch ScheduledTaskFinished
✅ Dispatch ScheduledTaskSucceeded or ScheduledTaskFailed depending on outcome

try {
    $event->run($this->laravel);
    $this->eventsRan = true;

    if (! $event->runInBackground) {
        if ($event->exitCode != 0) {
            throw new Exception("Scheduled command [{$event->command}] failed with exit code [{$event->exitCode}].");
        }

        $this->dispatcher->dispatch(new ScheduledTaskSucceeded(
            $event,
            round(microtime(true) - $start, 2)
        ));
    }

} catch (Throwable $e) {
    $this->dispatcher->dispatch(new ScheduledTaskFailed($event, $e));
    $this->handler->report($e);
} finally {
    $this->dispatcher->dispatch(new ScheduledTaskFinished(
        $event,
        round(microtime(true) - $start, 2)
    ));
}

✅ Pros:

Always logs that the task has finished, regardless of success or failure.
Separates clearly between succeeded and failed states.

❌ Cons:

May log ScheduledTaskFinished even after a ScheduledTaskFailed, potentially duplicating effort.

2️⃣ Second Proposition

✅ Dispatch Succeeded and Finished only if no exception is thrown
❌ No Finished if the task fails

try {
    $event->run($this->laravel);
    $this->eventsRan = true;

    if (! $event->runInBackground) {
        if ($event->exitCode != 0) {
            throw new Exception("Scheduled command [{$event->command}] failed with exit code [{$event->exitCode}].");
        }

        $this->dispatcher->dispatch(new ScheduledTaskSucceeded(
            $event,
            round(microtime(true) - $start, 2)
        ));
    }

    $this->dispatcher->dispatch(new ScheduledTaskFinished(
        $event,
        round(microtime(true) - $start, 2)
    ));
} catch (Throwable $e) {
    $this->dispatcher->dispatch(new ScheduledTaskFailed($event, $e));
    $this->handler->report($e);
}

✅ Pros:

Avoids unnecessary Finished dispatch after failure.

❌ Cons:

Makes it hard to track task completion if it failed (no Finished event).
Ambiguity in logs or observers about whether a task ever completed.

3️⃣ Third Proposition

❌ No Succeeded dispatch at all
✅ Dispatch Finished only if no exception is thrown

try {
    $event->run($this->laravel);
    $this->eventsRan = true;

    if (! $event->runInBackground) {
        if ($event->exitCode != 0) {
            throw new Exception("Scheduled command [{$event->command}] failed with exit code [{$event->exitCode}].");
        }
    }

    $this->dispatcher->dispatch(new ScheduledTaskFinished(
        $event,
        round(microtime(true) - $start, 2)
    ));
} catch (Throwable $e) {
    $this->dispatcher->dispatch(new ScheduledTaskFailed($event, $e));
    $this->handler->report($e);
}

✅ Pros:

Very minimalist.

❌ Cons:

No visibility into whether task succeeded—only that it didn’t fail.
Could be confusing for observers or logs.
Eliminates granularity between succeeded and finished.

✅ Recommendation

First Proposition is the most complete and traceable approach.

It ensures:

Every task emits a final Finished state.
It’s clear if a task succeeded or failed.
Observability and logging are optimal for debugging and monitoring.

This gives the best traceability, clean separation of concern, and ensures that listeners can react to the completion of a task—success or failure—without ambiguity.

🗣️ What do you think?

1 Should ScheduledTaskFinished always be dispatched, even after failure?
2 Do you rely on it in your cleanup, observability, or monitoring systems?
Would changing this behavior break your workflow?

Looking forward to your thoughts and use cases.

@achrafAa

What do you think should be the better approach?

1. Should `ScheduledTaskFinished` always be dispatched, even after failure? and add `ScheduledTaskSucceeded`

100%

2. Dispatch `ScheduledTaskSucceeded` and `ScheduledTaskFinished` only if no exception is thrown

0%

3. Dispatch `ScheduledTaskFinished` only if no exception is thrown, No `ScheduledTaskSucceeded` implementation

0%

1 vote

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Should ScheduledTaskFinished Be Dispatched When the Task Fails? #55723

{{title}}

Replies: 0 comments

Select a reply

Should ScheduledTaskFinished Be Dispatched When the Task Fails? #55723

achrafAa May 13, 2025

The Question

Context

Options Considered

1️⃣ First Proposition

✅ Pros:

❌ Cons:

2️⃣ Second Proposition

✅ Pros:

❌ Cons:

3️⃣ Third Proposition

✅ Pros:

❌ Cons:

✅ Recommendation

🗣️ What do you think?

Replies: 0 comments

achrafAa
May 13, 2025