diff --git a/app/Jobs/Schedule/RunTaskJob.php b/app/Jobs/Schedule/RunTaskJob.php index fe44058a0..8784a37cb 100644 --- a/app/Jobs/Schedule/RunTaskJob.php +++ b/app/Jobs/Schedule/RunTaskJob.php @@ -7,11 +7,12 @@ use Pterodactyl\Jobs\Job; use Carbon\CarbonImmutable; use Pterodactyl\Models\Task; use InvalidArgumentException; +use Pterodactyl\Models\Schedule; +use Illuminate\Support\Facades\Log; use Illuminate\Queue\SerializesModels; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\DispatchesJobs; -use Pterodactyl\Repositories\Eloquent\TaskRepository; use Pterodactyl\Services\Backups\InitiateBackupService; use Pterodactyl\Repositories\Wings\DaemonPowerRepository; use Pterodactyl\Repositories\Wings\DaemonCommandRepository; diff --git a/app/Services/Schedules/ProcessScheduleService.php b/app/Services/Schedules/ProcessScheduleService.php index ec16bc36a..86775430b 100644 --- a/app/Services/Schedules/ProcessScheduleService.php +++ b/app/Services/Schedules/ProcessScheduleService.php @@ -2,6 +2,7 @@ namespace Pterodactyl\Services\Schedules; +use Exception; use Pterodactyl\Models\Schedule; use Illuminate\Contracts\Bus\Dispatcher; use Pterodactyl\Jobs\Schedule\RunTaskJob; @@ -60,8 +61,22 @@ class ProcessScheduleService $task->update(['is_queued' => true]); }); - $this->dispatcher->{$now ? 'dispatchNow' : 'dispatch'}( - (new RunTaskJob($task))->delay($task->time_offset) - ); + $job = new RunTaskJob($task); + + if (! $now) { + $this->dispatcher->dispatch($job->delay($task->time_offset)); + } else { + // When using dispatchNow the RunTaskJob::failed() function is not called automatically + // so we need to manually trigger it and then continue with the exception throw. + // + // @see https://github.com/pterodactyl/panel/issues/2550 + try { + $this->dispatcher->dispatchNow($job); + } catch (Exception $exception) { + $job->failed($exception); + + throw $exception; + } + } } } diff --git a/tests/Integration/Api/Client/Server/Schedule/ExecuteScheduleTest.php b/tests/Integration/Api/Client/Server/Schedule/ExecuteScheduleTest.php index 00c57d4a4..d83faae3c 100644 --- a/tests/Integration/Api/Client/Server/Schedule/ExecuteScheduleTest.php +++ b/tests/Integration/Api/Client/Server/Schedule/ExecuteScheduleTest.php @@ -44,7 +44,8 @@ class ExecuteScheduleTest extends ClientApiIntegrationTestCase $this->actingAs($user)->postJson($this->link($schedule, '/execute'))->assertStatus(Response::HTTP_ACCEPTED); Bus::assertDispatched(function (RunTaskJob $job) use ($task) { - $this->assertSame($task->time_offset, $job->delay); + // A task executed right now should not have any job delay associated with it. + $this->assertNull($job->delay); $this->assertSame($task->id, $job->task->id); return true; diff --git a/tests/Integration/Services/Schedules/ProcessScheduleServiceTest.php b/tests/Integration/Services/Schedules/ProcessScheduleServiceTest.php index a5a4f65f3..93c793d6c 100644 --- a/tests/Integration/Services/Schedules/ProcessScheduleServiceTest.php +++ b/tests/Integration/Services/Schedules/ProcessScheduleServiceTest.php @@ -3,6 +3,8 @@ namespace Pterodactyl\Tests\Integration\Services\Schedules; use Mockery; +use Exception; +use Carbon\CarbonImmutable; use Pterodactyl\Models\Task; use InvalidArgumentException; use Pterodactyl\Models\Schedule; @@ -61,7 +63,7 @@ class ProcessScheduleServiceTest extends IntegrationTestCase */ public function testJobCanBeDispatchedWithExpectedInitialDelay($now) { - $this->swap(Dispatcher::class, $dispatcher = Mockery::mock(Dispatcher::class)); + Bus::fake(); $server = $this->createServerModel(); @@ -71,12 +73,17 @@ class ProcessScheduleServiceTest extends IntegrationTestCase /** @var \Pterodactyl\Models\Task $task */ $task = factory(Task::class)->create(['schedule_id' => $schedule->id, 'time_offset' => 10, 'sequence_id' => 1]); - $dispatcher->expects($now ? 'dispatchNow' : 'dispatch')->with(Mockery::on(function (RunTaskJob $job) use ($task) { - return $task->id === $job->task->id && $job->delay === 10; - })); - $this->getService()->handle($schedule, $now); + Bus::assertDispatched(RunTaskJob::class, function ($job) use ($now, $task) { + $this->assertInstanceOf(RunTaskJob::class, $job); + $this->assertSame($task->id, $job->task->id); + // Jobs using dispatchNow should not have a delay associated with them. + $this->assertSame($now ? null : 10, $job->delay); + + return true; + }); + $this->assertDatabaseHas('schedules', ['id' => $schedule->id, 'is_processing' => true]); $this->assertDatabaseHas('tasks', ['id' => $task->id, 'is_queued' => true]); } @@ -89,7 +96,7 @@ class ProcessScheduleServiceTest extends IntegrationTestCase */ public function testFirstSequenceTaskIsFound() { - $this->swap(Dispatcher::class, $dispatcher = Mockery::mock(Dispatcher::class)); + Bus::fake(); $server = $this->createServerModel(); /** @var \Pterodactyl\Models\Schedule $schedule */ @@ -100,18 +107,50 @@ class ProcessScheduleServiceTest extends IntegrationTestCase $task = factory(Task::class)->create(['schedule_id' => $schedule->id, 'sequence_id' => 2]); $task3 = factory(Task::class)->create(['schedule_id' => $schedule->id, 'sequence_id' => 3]); - $dispatcher->expects('dispatch')->with(Mockery::on(function (RunTaskJob $job) use ($task) { - return $task->id === $job->task->id; - })); - $this->getService()->handle($schedule); + Bus::assertDispatched(RunTaskJob::class, function (RunTaskJob $job) use ($task) { + return $task->id === $job->task->id; + }); + $this->assertDatabaseHas('schedules', ['id' => $schedule->id, 'is_processing' => true]); $this->assertDatabaseHas('tasks', ['id' => $task->id, 'is_queued' => true]); $this->assertDatabaseHas('tasks', ['id' => $task2->id, 'is_queued' => false]); $this->assertDatabaseHas('tasks', ['id' => $task3->id, 'is_queued' => false]); } + /** + * Tests that a task's processing state is reset correctly if using "dispatchNow" and there is + * an exception encountered while running it. + * + * @see https://github.com/pterodactyl/panel/issues/2550 + */ + public function testTaskDispatchedNowIsResetProperlyIfErrorIsEncountered() + { + $this->swap(Dispatcher::class, $dispatcher = Mockery::mock(Dispatcher::class)); + + $server = $this->createServerModel(); + /** @var \Pterodactyl\Models\Schedule $schedule */ + $schedule = factory(Schedule::class)->create(['server_id' => $server->id, 'last_run_at' => null]); + /** @var \Pterodactyl\Models\Task $task */ + $task = factory(Task::class)->create(['schedule_id' => $schedule->id, 'sequence_id' => 1]); + + $dispatcher->expects('dispatchNow')->andThrows(new Exception('Test thrown exception')); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('Test thrown exception'); + + $this->getService()->handle($schedule, true); + + $this->assertDatabaseHas('schedules', [ + 'id' => $schedule->id, + 'is_processing' => false, + 'last_run_at' => CarbonImmutable::now()->toAtomString(), + ]); + + $this->assertDatabaseHas('tasks', ['id' => $task->id, 'is_queued' => false]); + } + /** * @return array */