Correctly reset a schedule if there is an exception during the run stage; closes pterodactyl#2550

Author: DaneEveritt

app/Jobs/Schedule/RunTaskJob.php

@@ -7,11 +7,12 @@
 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;

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 @@ public function handle(Schedule $schedule, bool $now = false)
             $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;
+            }
+        }
     }
 }

tests/Integration/Api/Client/Server/Schedule/ExecuteScheduleTest.php

@@ -44,7 +44,8 @@ public function testScheduleIsExecutedRightAway(array $permissions)
         $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;

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 @@ public function testErrorDuringScheduleDataUpdateDoesNotPersistChanges()
      */
     public function testJobCanBeDispatchedWithExpectedInitialDelay($now)
     {
-        $this->swap(Dispatcher::class, $dispatcher = Mockery::mock(Dispatcher::class));
+        Bus::fake();
 
         $server = $this->createServerModel();
 
@@ -71,12 +73,17 @@ public function testJobCanBeDispatchedWithExpectedInitialDelay($now)
         /** @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 @@ public function testJobCanBeDispatchedWithExpectedInitialDelay($now)
      */
     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 @@ public function testFirstSequenceTaskIsFound()
         $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
      */