Merge pull request #6153 from betagouv/retry-active-storage-jobs-on-transient-error

Les jobs ActiveStorage et ApplicationJob ré-essaient automatiquement plus tard en cas d'erreur réseau (#6153)

app/jobs/application_job.rb

@@ -1,7 +1,7 @@
 class ApplicationJob < ActiveJob::Base
-  DEFAULT_MAX_ATTEMPTS_JOBS = 25
+  include ActiveJob::RetryOnTransientErrors
 
-  retry_on ::Excon::Error::BadRequest
+  DEFAULT_MAX_ATTEMPTS_JOBS = 25
 
   before_perform do |job|
     Rails.logger.info("#{job.class.name} started at #{Time.zone.now}")

app/lib/active_job/retry_on_transient_errors.rb

@@ -0,0 +1,17 @@
+module ActiveJob::RetryOnTransientErrors
+  extend ActiveSupport::Concern
+
+  TRANSIENT_ERRORS = [
+    Excon::Error::InternalServerError,
+    Excon::Error::GatewayTimeout,
+    Excon::Error::BadRequest
+  ]
+
+  included do
+    if handler_for_rescue(TRANSIENT_ERRORS.first).nil?
+      TRANSIENT_ERRORS.each do |error_type|
+        retry_on error_type, attempts: 5, wait: :exponentially_longer
+      end
+    end
+  end
+end

config/initializers/active_storage.rb

@@ -14,6 +14,12 @@ ActiveSupport.on_load(:active_storage_attachment) do
   include AttachmentVirusScannerConcern
 end
 
+Rails.application.reloader.to_prepare do
+  class ActiveStorage::BaseJob
+    include ActiveJob::RetryOnTransientErrors
+  end
+end
+
 # When an OpenStack service is initialized it makes a request to fetch
 # `publicURL` to use for all operations. We intercept the method that reads
 # this url and replace the host with DS_Proxy host. This way all the operation

spec/jobs/active_storage/base_job_spec.rb

@@ -0,0 +1,3 @@
+describe ActiveStorage::BaseJob do
+  it_behaves_like 'a job retrying transient errors'
+end

spec/jobs/application_job_spec.rb

@@ -1,6 +1,8 @@
 include ActiveJob::TestHelper
 
 RSpec.describe ApplicationJob, type: :job do
+  it_behaves_like 'a job retrying transient errors'
+
   describe 'perform' do
     before do
       allow(Rails.logger).to receive(:info)
@@ -13,23 +15,7 @@ RSpec.describe ApplicationJob, type: :job do
     end
   end
 
-  context 'when ::Excon::Error::BadRequest is raised' do
-    # https://api.rubyonrails.org/classes/ActiveJob/Exceptions/ClassMethods.html#method-i-retry_on
-    # retry on will try 5 times and then bubble up the error
-    it 'makes 5 attempts' do
-      assert_performed_jobs 5 do
-        ExconErrJob.perform_later rescue ::Excon::Error::BadRequest
-      end
-    end
-  end
-
   class ChildJob < ApplicationJob
     def perform; end
   end
-
-  class ExconErrJob < ApplicationJob
-    def perform
-      raise ::Excon::Error::BadRequest.new('bad request')
-    end
-  end
 end

spec/lib/active_job/retry_on_transient_errors_spec.rb

@@ -0,0 +1,9 @@
+describe ActiveJob::RetryOnTransientErrors do
+  # rubocop:disable Rails/ApplicationJob
+  class Job < ActiveJob::Base
+    include ActiveJob::RetryOnTransientErrors
+  end
+  # rubocop:enable Rails/ApplicationJob
+
+  it_behaves_like 'a job retrying transient errors', Job
+end

spec/support/shared_examples_for_jobs.rb

@@ -0,0 +1,29 @@
+RSpec.shared_examples 'a job retrying transient errors' do |job_class = described_class|
+  context 'when a transient network error is raised' do
+    ExconErrorJob = Class.new(job_class) do
+      def perform
+        raise Excon::Error::InternalServerError, 'msg'
+      end
+    end
+
+    it 'makes 5 attempts before raising the exception up' do
+      assert_performed_jobs 5 do
+        ExconErrorJob.perform_later rescue Excon::Error::InternalServerError
+      end
+    end
+  end
+
+  context 'when another type of error is raised' do
+    StandardErrorJob = Class.new(job_class) do
+      def perform
+        raise StandardError
+      end
+    end
+
+    it 'makes only 1 attempt before raising the exception up' do
+      assert_performed_jobs 1 do
+        StandardErrorJob.perform_later rescue StandardError
+      end
+    end
+  end
+end