[57932] Make Mails::MailerJob retry failed emails

app/workers/application_job.rb

@@ -30,13 +30,7 @@
 
 class ApplicationJob < ActiveJob::Base
   include ::JobStatus::ApplicationJobWithStatus
-
-  ##
-  # By default, do not log the arguments of a background job
-  # to avoid leaking sensitive information to logs
-  self.log_arguments = false
-
-  around_perform :prepare_job_context
+  include SharedJobSetup
 
   ##
   # Return a priority number on the given payload
@@ -65,45 +59,7 @@ def self.queue_with_priority(value = :default)
     end
   end
 
-  # Resets the thread local request store.
-  # This should be done, because normal application code expects the RequestStore to be
-  # invalidated between multiple requests and does usually not care whether it is executed
-  # from a request or from a delayed job.
-  # For a delayed job, each job execution is the thing that comes closest to
-  # the concept of a new request.
-  def with_clean_request_store
-    store = RequestStore.store
-
-    begin
-      RequestStore.clear!
-      yield
-    ensure
-      # Reset to previous value
-      RequestStore.clear!
-      RequestStore.store.merge! store
-    end
-  end
-
-  # Reloads the thread local ActionMailer configuration.
-  # Since the email configuration is now done in the web app, we need to
-  # make sure that any changes to the configuration is correctly picked up
-  # by the background jobs at runtime.
-  def reload_mailer_settings!
-    Setting.reload_mailer_settings!
-  end
-
   def job_scheduled_at
     GoodJob::Job.where(id: job_id).pick(:scheduled_at)
   end
-
-  private
-
-  def prepare_job_context
-    with_clean_request_store do
-      ::OpenProject::Appsignal.tag_request
-      reload_mailer_settings!
-
-      yield
-    end
-  end
 end

app/workers/mails/mailer_job.rb

@@ -26,46 +26,44 @@
 # See COPYRIGHT and LICENSE files for more details.
 #++
 
-##
-# This job gets called when internally using
+# OpenProject is configured to use this job when sending emails like this:
 #
 # ```
 # UserMailer.some_mail("some param").deliver_later
 # ```
 #
-# because we want to have the sending of the email run in an `ApplicationJob`
-# as opposed to using `ActiveJob::QueueAdapters::DelayedJobAdapter::JobWrapper`.
-# We want it to run in an `ApplicationJob` because of the shared setup required
-# such as reloading the mailer configuration and resetting the request store.
-class Mails::MailerJob < ApplicationJob
-  queue_as { ActionMailer::Base.deliver_later_queue_name }
-
-  # Retry mailing jobs three times with polinomial backoff
-  retry_on StandardError, wait: :polynomially_longer, attempts: 3
-
-  # If exception is handled in mail handler
-  # retry_on will be ignored
-  rescue_from StandardError, with: :handle_exception_with_mailer_class
-
-  def perform(mailer, mail_method, delivery, args:)
-    mailer.constantize.public_send(mail_method, *args).send(delivery)
-  end
-
-  private
-
-  # "Deserialize" the mailer class name by hand in case another argument
-  # (like a Global ID reference) raised DeserializationError.
-  def mailer_class
-    if mailer = Array(@serialized_arguments).first || Array(arguments).first
-      mailer.constantize
-    end
-  end
+# This job is used because all our `XxxMailer` classes inherit from
+# `ApplicationMailer`, and `ApplicationMailer.delivery_job` is set to
+# `::Mails::MailerJob`.
+#
+# The `delivery_job` is customized to add the shared job setup required for
+# OpenProject such as reloading the mailer configuration and resetting the
+# request store on each job execution.
+#
+# It also adds retry logic to the job.
+class Mails::MailerJob < ActionMailer::MailDeliveryJob
+  include SharedJobSetup
 
-  def handle_exception_with_mailer_class(exception)
-    if klass = mailer_class
-      klass.handle_exception exception
-    else
-      raise exception
-    end
-  end
+  # Retry mailing jobs 14 times with polynomial backoff (retries for ~ 1.5 days).
+  #
+  # with polynomial backoff, the formula to get wait_duration is:
+  #
+  #   ((executions**4) + (Kernel.rand * (executions**4) * jitter)) + 2
+  #
+  # as the default jitter is 0.0, the formula becomes:
+  #
+  #   ((executions**4) + 2)
+  #
+  # To get the numbers, run this:
+  #
+  #     (1..20).reduce(0) do |total_wait, i|
+  #       wait = (i**4) + 2
+  #       total_wait += wait
+  #       puts "Execution #{i} waits #{wait} secs. Total wait: #{total_wait} secs"
+  #       total_wait
+  #     end
+  #
+  # We set attemps to 14 to have it retry for 127715 seconds which is more than
+  # 1 day (~= 1 day 11 hours 30 min)
+  retry_on StandardError, wait: :polynomially_longer, attempts: 14
 end

app/workers/shared_job_setup.rb

@@ -0,0 +1,92 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+# Shared setup for jobs.
+#
+# This module is included in `ApplicationJob` and `Mails::MailerJob` and does
+# the following:
+#
+#   - disable logging of arguments
+#   - before each job execution:
+#     - reloads the mailer settings
+#     - resets the request store
+#     - tags the request for AppSignal
+module SharedJobSetup
+  extend ActiveSupport::Concern
+
+  included do
+    # By default, do not log the arguments of a background job
+    # to avoid leaking sensitive information to logs
+    self.log_arguments = false
+
+    around_perform :prepare_job_context
+  end
+
+  # Prepare the job execution by cleaning the request store, reloading the
+  # mailer settings and tagging the request
+  def prepare_job_context
+    with_clean_request_store do
+      ::OpenProject::Appsignal.tag_request
+      reload_mailer_settings!
+
+      yield
+    end
+  end
+
+  # Resets the thread local request store.
+  #
+  # This should be done, because normal application code expects the
+  # RequestStore to be invalidated between multiple requests and does usually
+  # not care whether it is executed from a request or from a job.
+  #
+  # For a job, each job execution is the thing that comes closest to the concept
+  # of a new request.
+  def with_clean_request_store
+    store = RequestStore.store
+
+    begin
+      RequestStore.clear!
+      yield
+    ensure
+      # Reset to previous value
+      RequestStore.clear!
+      RequestStore.store.merge! store
+    end
+  end
+
+  # Reloads the thread local ActionMailer configuration.
+  #
+  # Since the email configuration is done in the web app, it makes sure that any
+  # changes to the configuration is correctly picked up by the background jobs
+  # at runtime.
+  def reload_mailer_settings!
+    Setting.reload_mailer_settings!
+  end
+end