Ensure that report helper is called when a batch is failed (#696)

* test: assert that report helper is called when a batch is failed

* fix: make sure report method is called

* chore: fix indent

* refactor: move logic into isolated class methods

* doc: add CHANGELOG

Author: m90

CHANGELOG.md

@@ -1,6 +1,9 @@
 # api
 
 
+## 8x.28.9 - 28 November 2023
+- Fix error reporting on detecting failed Qs Batches
+
 ## 8x.28.8 - 28 November 2023
 - Chunk query for all page update events in QsBatches job
 - Reenable QsBatches job

app/Jobs/RequeuePendingQsBatchesJob.php

@@ -18,23 +18,40 @@ public function __construct() {
 
     public function handle(): void
     {
-        DB::transaction(function () {
-            tap(QsBatch::where([
+        $failedBatches = $this->markBatchesFailed();
+        foreach ($failedBatches as $batchId) {
+            report("QsBatch with ID ".$batchId."was marked as failed.");
+        }
+
+        $this->requeueStalledBatches();
+    }
+
+    private function markBatchesFailed(): array
+    {
+        return DB::transaction(function () {
+            $failedBatches = QsBatch::where([
                 ['processing_attempts', '>=', $this->markFailedAfter],
                 ['failed', '=', false],
-            ]))
-                ->update(['failed' => true])
+            ])
+                ->select('id')
                 ->get()
-                ->each(function ($batch, $index) {
-                    report("QsBatch with ID ".$batch->id."was marked as failed.");
-                });
-
-            $threshold = Carbon::now()->subtract(new \DateInterval($this->pendingTimeout));
-            QsBatch::where([['pending_since', '<>', null], ['pending_since', '<', $threshold]])
-                ->increment(
-                    'processing_attempts', 1,
-                    ['pending_since' => null, 'done' => 0]
-                );
+                ->pluck('id')
+                ->toArray();
+            QsBatch::whereIn('id', $failedBatches)->update(['failed' => true]);
+            return $failedBatches;
         });
     }
+
+    private function requeueStalledBatches(): void
+    {
+        $threshold = Carbon::now()->subtract(new \DateInterval($this->pendingTimeout));
+        QsBatch::where([
+            ['pending_since', '<>', null],
+            ['pending_since', '<', $threshold],
+        ])
+            ->increment(
+                'processing_attempts', 1,
+                ['pending_since' => null, 'done' => 0]
+            );
+    }
 }

tests/Jobs/RequeuePendingQsBatchesJobTest.php

@@ -8,6 +8,7 @@
 use Illuminate\Foundation\Testing\RefreshDatabase;
 use Illuminate\Contracts\Queue\Job;
 use Carbon\Carbon;
+use Illuminate\Contracts\Debug\ExceptionHandler;
 
 class RequeuePendingQsBatchesJobTest extends TestCase
 {
@@ -30,6 +31,12 @@ public function testRequeue (): void {
         QsBatch::factory()->create(['pending_since' => Carbon::now()->subSeconds(400), 'id' => 2, 'done' => 0, 'wiki_id' => 1, 'entityIds' => 'a,b']);
         QsBatch::factory()->create(['processing_attempts' => 3, 'id' => 3, 'done' => 0, 'wiki_id' => 1, 'entityIds' => 'a,b']);
 
+        $mockExceptionHandler = $this->createMock(ExceptionHandler::class);
+        $mockExceptionHandler
+            ->expects($this->once())
+            ->method('report');
+        $this->app->instance(ExceptionHandler::class, $mockExceptionHandler);
+
         $mockJob = $this->createMock(Job::class);
         $job = new RequeuePendingQsBatchesJob();
         $job->setJob($mockJob);