Let Cloud Tasks handle task deletion

Author: marickvantuil

src/CloudTasksJob.php

@@ -69,24 +69,9 @@ public function getTaskName(): string
 
     public function delete(): void
     {
-        // Laravel automatically calls delete() after a job is processed successfully. However, this is
-        // not what we want to happen in Cloud Tasks because Cloud Tasks will also delete the task upon
-        // a 200 OK status, which means a task is deleted twice, possibly resulting in errors. So if the
-        // task was processed successfully (no errors or failures) then we will not delete the task
-        // manually and will let Cloud Tasks do it.
-        $successful =
-            // If the task has failed, we should be able to delete it permanently
-            $this->hasFailed() === false
-            // If the task has errored, it should be released, which in process deletes the errored task
-            && $this->hasError() === false;
-
-        if ($successful) {
-            return;
-        }
-
-        parent::delete();
-
-        $this->driver->delete($this);
+        // Laravel automatically calls delete() after a job is processed successfully.
+        // However, this is not what we want to happen in Cloud Tasks because Cloud Tasks
+        // will also delete the task upon a 200 OK status, which means a task is deleted twice.
     }
 
     public function hasError(): bool

src/CloudTasksQueue.php

@@ -222,13 +222,11 @@ public function delete(CloudTasksJob $job): void
 
     public function release(CloudTasksJob $job, int $delay = 0): void
     {
-        $job->delete();
-
-        $payload = $job->getRawBody();
-
-        $options = ['delay' => $delay];
-
-        $this->pushRaw($payload, $job->getQueue(), $options);
+        $this->pushRaw(
+            payload: $job->getRawBody(),
+            queue: $job->getQueue(),
+            options: ['delay' => $delay]
+        );
     }
 
     public function getHandler(): string

src/IncomingTask.php

@@ -26,7 +26,7 @@ public static function fromJson(string $payload): self
         }
     }
 
-    public function isEmpty(): bool
+    public function isInvalid(): bool
     {
         return $this->task === [];
     }

src/TaskHandler.php

@@ -21,7 +21,7 @@ public function handle(?string $task = null): void
     {
         $task = IncomingTask::fromJson($task ?: request()->getContent());
 
-        if ($task->isEmpty()) {
+        if ($task->isInvalid()) {
             abort(422, 'Invalid task payload');
         }
 
@@ -33,7 +33,12 @@ public function handle(?string $task = null): void
 
         $this->config = is_array($config) ? $config : [];
 
-        $this->run($task);
+        // We want to catch any errors so we have more fine-grained control over
+        // how tasks are retried. Cloud Tasks will retry the task if a 5xx status
+        // is returned. Because we manually manage retries by releaseing jobs,
+        // we never want to return a 5xx status as that will result in duplicate
+        // job attempts.
+        rescue(fn () => $this->run($task), report: false);
     }
 
     private function run(IncomingTask $task): void

tests/Support/FailingJob.php

@@ -12,6 +12,7 @@ class FailingJob extends BaseJob
 
     public function handle()
     {
+        event(new JobOutput('FailingJob:running'));
         throw new Error('simulating a failing job');
     }
 

tests/TaskHandlerTest.php

@@ -78,33 +78,31 @@ public function after_max_attempts_it_will_log_to_failed_table()
     }
 
     #[Test]
-    public function after_max_attempts_it_will_delete_the_task()
+    public function after_max_attempts_it_will_no_longer_execute_the_task()
     {
         // Arrange
+        Event::fake([JobOutput::class]);
         $job = $this->dispatch(new FailingJob());
 
         // Act & Assert
         $releasedJob = $job->runAndGetReleasedJob();
-        CloudTasksApi::assertDeletedTaskCount(1);
-        CloudTasksApi::assertTaskDeleted($job->task->getName());
+        Event::assertDispatched(JobOutput::class, 1);
         $this->assertDatabaseCount('failed_jobs', 0);
 
         $releasedJob = $releasedJob->runAndGetReleasedJob();
-        CloudTasksApi::assertDeletedTaskCount(2);
-        CloudTasksApi::assertTaskDeleted($job->task->getName());
+        Event::assertDispatched(JobOutput::class, 2);
         $this->assertDatabaseCount('failed_jobs', 0);
 
         $releasedJob->run();
-        CloudTasksApi::assertDeletedTaskCount(3);
-        CloudTasksApi::assertTaskDeleted($job->task->getName());
+        Event::assertDispatched(JobOutput::class, 4);
         $this->assertDatabaseCount('failed_jobs', 1);
     }
 
     #[Test]
     #[TestWith([['now' => '2020-01-01 00:00:00', 'try_at' => '2020-01-01 00:00:00', 'should_fail' => false]])]
     #[TestWith([['now' => '2020-01-01 00:00:00', 'try_at' => '2020-01-01 00:04:59', 'should_fail' => false]])]
     #[TestWith([['now' => '2020-01-01 00:00:00', 'try_at' => '2020-01-01 00:05:00', 'should_fail' => true]])]
-    public function after_max_retry_until_it_will_log_to_failed_table_and_delete_the_task(array $args)
+    public function after_max_retry_until_it_will_log_to_failed_table(array $args)
     {
         // Arrange
         $this->travelTo($args['now']);
@@ -115,8 +113,6 @@ public function after_max_retry_until_it_will_log_to_failed_table_and_delete_the
         $releasedJob = $job->runAndGetReleasedJob();
 
         // Assert
-        CloudTasksApi::assertDeletedTaskCount(1);
-        CloudTasksApi::assertTaskDeleted($job->task->getName());
         $this->assertDatabaseCount('failed_jobs', 0);
 
         // Act
@@ -130,6 +126,9 @@ public function after_max_retry_until_it_will_log_to_failed_table_and_delete_the
     #[Test]
     public function test_unlimited_max_attempts()
     {
+        // Assert
+        Event::fake(JobOutput::class);
+
         // Act
         $job = $this->dispatch(new FailingJobWithUnlimitedTries());
 
@@ -138,8 +137,7 @@ public function test_unlimited_max_attempts()
             $job = $job->runAndGetReleasedJob();
         }
 
-        // -1 because the last job is not run.
-        CloudTasksApi::assertDeletedTaskCount(51);
+        Event::assertDispatched(JobOutput::class, 51);
     }
 
     #[Test]
@@ -204,9 +202,7 @@ public function failing_jobs_are_released()
 
         $job->run();
 
-        CloudTasksApi::assertDeletedTaskCount(1);
         CloudTasksApi::assertCreatedTaskCount(2);
-        CloudTasksApi::assertTaskDeleted($job->task->getName());
         Event::assertDispatched(JobReleasedAfterException::class, function ($event) {
             return $event->job->attempts() === 1;
         });