Merge pull request #148 from biigle/patch-1

Restore behavior to ignore special exceptions
biigle · Feb 6, 2024 · b198fb0 · b198fb0
2 parents 8c38244 + e040202
commit b198fb0
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 4 deletions.
diff --git a/src/Jobs/ProcessAnnotatedFile.php b/src/Jobs/ProcessAnnotatedFile.php
@@ -128,7 +128,10 @@ public function handle()
                 ->onQueue($this->queue)
                 ->delay(60);
         } catch (Exception $e) {
-            if ($this->shouldRetryAfterException($e)) {
+            if ($this->shouldGiveUpAfterException($e)) {
+                $class = get_class($this->file);
+                Log::warning("Could not process annotated {$class} {$this->file->id}: {$e->getMessage()}", ['exception' => $e]);
+            } elseif ($this->shouldRetryAfterException($e)) {
                 // Exponential backoff for retry after 10 and then 20 minutes.
                 $this->release($this->attempts() * 600);
             } else {
@@ -147,13 +150,29 @@ public function handle()
     abstract public function handleFile(VolumeFile $file, $path);
 
     /**
-     * Determine if this job should retry instead of fail after an exception
+     * Determine if the job should give up trying because the error will likely not be
+     * fixed the next time.
      *
      * @param Exception $e
+     */
+    protected function shouldGiveUpAfterException(Exception $e): bool
+    {
+        $message = $e->getMessage();
+        $giveUpError = (
+            // SSL certificate problem of the remote server.
+            // See: https://curl.haxx.se/libcurl/c/libcurl-errors.html
+            Str::contains($message, 'cURL error 60:')
+        );
+
+        return $giveUpError;
+    }
+
+    /**
+     * Determine if this job should retry instead of fail after an exception
      *
-     * @return bool
+     * @param Exception $e
      */
-    protected function shouldRetryAfterException(Exception $e)
+    protected function shouldRetryAfterException(Exception $e): bool
     {
         $message = $e->getMessage();
         $knownError = (

diff --git a/tests/Jobs/ProcessAnnotatedImageTest.php b/tests/Jobs/ProcessAnnotatedImageTest.php
@@ -15,6 +15,7 @@
 use File;
 use FileCache;
 use Jcupitt\Vips\Image as VipsImage;
+use Log;
 use Mockery;
 use Storage;
 use TestCase;
@@ -385,6 +386,19 @@ public function testHandleError()
         }
     }
 
+    public function testHandleGiveUpError()
+    {
+        $disk = Storage::fake('test');
+        FileCache::shouldReceive('get')->andThrow(new Exception('cURL error 60:'));
+        Log::shouldReceive('warning')->once();
+
+        $annotation = ImageAnnotationTest::create();
+        $job = new ProcessAnnotatedImage($annotation->image);
+        $job->handle();
+        $prefix = fragment_uuid_path($annotation->image->uuid);
+        $disk->assertMissing("{$prefix}/{$annotation->id}.svg");
+    }
+
     public function testFileLockedError()
     {
         $disk = Storage::fake('test');

diff --git a/tests/Jobs/ProcessAnnotatedVideoTest.php b/tests/Jobs/ProcessAnnotatedVideoTest.php
@@ -429,6 +429,19 @@ public function testHandleError()
         }
     }
 
+    public function testHandleGiveUpError()
+    {
+        $disk = Storage::fake('test');
+        FileCache::shouldReceive('get')->andThrow(new Exception('cURL error 60:'));
+        Log::shouldReceive('warning')->once();
+
+        $annotation = VideoAnnotationTest::create();
+        $job = new ProcessAnnotatedVideo($annotation->video);
+        $job->handle();
+        $prefix = fragment_uuid_path($annotation->video->uuid);
+        $disk->assertMissing("{$prefix}/v-{$annotation->id}.svg");
+    }
+
     public function testFileLockedError()
     {
         $disk = Storage::fake('test');