Skip to content

Commit c7a87ae

Browse files
authored
[12.x] ScheduledTaskFailed not dispatched on scheduled forground task fails (#55624)
* Add exception handling for failed scheduled commands - 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. * Add test for command without explicit return * add test for no explicit return * Revert "add test for no explicit return" This reverts commit cdc08e3. * Add tests for background command execution with and without explicit return * move the check after set-ting the eventsRan to true * redo pint removing doc blocks for handle
1 parent 8da277b commit c7a87ae

File tree

2 files changed

+218
-0
lines changed

2 files changed

+218
-0
lines changed

src/Illuminate/Console/Scheduling/ScheduleRunCommand.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace Illuminate\Console\Scheduling;
44

5+
use Exception;
56
use Illuminate\Console\Application;
67
use Illuminate\Console\Command;
78
use Illuminate\Console\Events\ScheduledTaskFailed;
@@ -197,6 +198,10 @@ protected function runEvent($event)
197198
));
198199

199200
$this->eventsRan = true;
201+
202+
if ($event->exitCode != 0 && ! $event->runInBackground) {
203+
throw new Exception("Scheduled command [{$event->command}] failed with exit code [{$event->exitCode}].");
204+
}
200205
} catch (Throwable $e) {
201206
$this->dispatcher->dispatch(new ScheduledTaskFailed($event, $e));
202207

Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
<?php
2+
3+
namespace Illuminate\Tests\Integration\Console\Scheduling;
4+
5+
use Illuminate\Console\Events\ScheduledTaskFailed;
6+
use Illuminate\Console\Events\ScheduledTaskFinished;
7+
use Illuminate\Console\Events\ScheduledTaskStarting;
8+
use Illuminate\Console\Scheduling\Schedule;
9+
use Illuminate\Contracts\Container\BindingResolutionException;
10+
use Illuminate\Support\Carbon;
11+
use Illuminate\Support\Facades\Event;
12+
use Orchestra\Testbench\TestCase;
13+
14+
class ScheduleRunCommandTest extends TestCase
15+
{
16+
protected function setUp(): void
17+
{
18+
parent::setUp();
19+
Carbon::setTestNow(Carbon::now());
20+
}
21+
22+
protected function tearDown(): void
23+
{
24+
parent::tearDown();
25+
Carbon::setTestNow();
26+
}
27+
28+
/**
29+
* @throws BindingResolutionException
30+
*/
31+
public function test_failing_command_in_foreground_triggers_event()
32+
{
33+
Event::fake([
34+
ScheduledTaskStarting::class,
35+
ScheduledTaskFinished::class,
36+
ScheduledTaskFailed::class,
37+
]);
38+
39+
// Create a schedule and add the command
40+
$schedule = $this->app->make(Schedule::class);
41+
$task = $schedule->exec('exit 1')
42+
->everyMinute();
43+
44+
// Make sure it will run regardless of schedule
45+
$task->when(function () {
46+
return true;
47+
});
48+
49+
// Execute the scheduler
50+
$this->artisan('schedule:run');
51+
52+
// Verify the event sequence
53+
Event::assertDispatched(ScheduledTaskStarting::class);
54+
Event::assertDispatched(ScheduledTaskFinished::class);
55+
Event::assertDispatched(ScheduledTaskFailed::class, function ($event) use ($task) {
56+
return $event->task === $task &&
57+
$event->exception->getMessage() === 'Scheduled command [exit 1] failed with exit code [1].';
58+
});
59+
}
60+
61+
/**
62+
* @throws BindingResolutionException
63+
*/
64+
public function test_failing_command_in_background_does_not_trigger_event()
65+
{
66+
Event::fake([
67+
ScheduledTaskStarting::class,
68+
ScheduledTaskFinished::class,
69+
ScheduledTaskFailed::class,
70+
]);
71+
72+
// Create a schedule and add the command
73+
$schedule = $this->app->make(Schedule::class);
74+
$task = $schedule->exec('exit 1')
75+
->everyMinute()
76+
->runInBackground();
77+
78+
// Make sure it will run regardless of schedule
79+
$task->when(function () {
80+
return true;
81+
});
82+
83+
// Execute the scheduler
84+
$this->artisan('schedule:run');
85+
86+
// Verify the event sequence
87+
Event::assertDispatched(ScheduledTaskStarting::class);
88+
Event::assertDispatched(ScheduledTaskFinished::class);
89+
Event::assertNotDispatched(ScheduledTaskFailed::class);
90+
}
91+
92+
/**
93+
* @throws BindingResolutionException
94+
*/
95+
public function test_successful_command_does_not_trigger_event()
96+
{
97+
Event::fake([
98+
ScheduledTaskStarting::class,
99+
ScheduledTaskFinished::class,
100+
ScheduledTaskFailed::class,
101+
]);
102+
103+
// Create a schedule and add the command
104+
$schedule = $this->app->make(Schedule::class);
105+
$task = $schedule->exec('exit 0')
106+
->everyMinute();
107+
108+
// Make sure it will run regardless of schedule
109+
$task->when(function () {
110+
return true;
111+
});
112+
113+
// Execute the scheduler
114+
$this->artisan('schedule:run');
115+
116+
// Verify the event sequence
117+
Event::assertDispatched(ScheduledTaskStarting::class);
118+
Event::assertDispatched(ScheduledTaskFinished::class);
119+
Event::assertNotDispatched(ScheduledTaskFailed::class);
120+
}
121+
122+
/**
123+
* @throws BindingResolutionException
124+
*/
125+
public function test_command_with_no_explicit_return_does_not_trigger_event()
126+
{
127+
Event::fake([
128+
ScheduledTaskStarting::class,
129+
ScheduledTaskFinished::class,
130+
ScheduledTaskFailed::class,
131+
]);
132+
133+
// Create a schedule and add the command that just performs an action without explicit exit
134+
$schedule = $this->app->make(Schedule::class);
135+
$task = $schedule->exec('true')
136+
->everyMinute();
137+
138+
// Make sure it will run regardless of schedule
139+
$task->when(function () {
140+
return true;
141+
});
142+
143+
// Execute the scheduler
144+
$this->artisan('schedule:run');
145+
146+
// Verify the event sequence
147+
Event::assertDispatched(ScheduledTaskStarting::class);
148+
Event::assertDispatched(ScheduledTaskFinished::class);
149+
Event::assertNotDispatched(ScheduledTaskFailed::class);
150+
}
151+
152+
/**
153+
* @throws BindingResolutionException
154+
*/
155+
public function test_successful_command_in_background_does_not_trigger_event()
156+
{
157+
Event::fake([
158+
ScheduledTaskStarting::class,
159+
ScheduledTaskFinished::class,
160+
ScheduledTaskFailed::class,
161+
]);
162+
163+
// Create a schedule and add the command
164+
$schedule = $this->app->make(Schedule::class);
165+
$task = $schedule->exec('exit 0')
166+
->everyMinute()
167+
->runInBackground();
168+
169+
// Make sure it will run regardless of schedule
170+
$task->when(function () {
171+
return true;
172+
});
173+
174+
// Execute the scheduler
175+
$this->artisan('schedule:run');
176+
177+
// Verify the event sequence
178+
Event::assertDispatched(ScheduledTaskStarting::class);
179+
Event::assertDispatched(ScheduledTaskFinished::class);
180+
Event::assertNotDispatched(ScheduledTaskFailed::class);
181+
}
182+
183+
/**
184+
* @throws BindingResolutionException
185+
*/
186+
public function test_command_with_no_explicit_return_in_background_does_not_trigger_event()
187+
{
188+
Event::fake([
189+
ScheduledTaskStarting::class,
190+
ScheduledTaskFinished::class,
191+
ScheduledTaskFailed::class,
192+
]);
193+
194+
// Create a schedule and add the command that just performs an action without explicit exit
195+
$schedule = $this->app->make(Schedule::class);
196+
$task = $schedule->exec('true')
197+
->everyMinute()
198+
->runInBackground();
199+
200+
// Make sure it will run regardless of schedule
201+
$task->when(function () {
202+
return true;
203+
});
204+
205+
// Execute the scheduler
206+
$this->artisan('schedule:run');
207+
208+
// Verify the event sequence
209+
Event::assertDispatched(ScheduledTaskStarting::class);
210+
Event::assertDispatched(ScheduledTaskFinished::class);
211+
Event::assertNotDispatched(ScheduledTaskFailed::class);
212+
}
213+
}

0 commit comments

Comments
 (0)