feat(mailer): make errors interceptable by jobs so they will retry

Comme on intercepte chaque `StandardError` pour le monitoring des mails
en erreur, l'erreur n'était plus visible par le job, et les emails
étaient perdus.

A la place on re-raise une autre erreur pour que le job échoue afin de
retry plus tard. Pour ne pas être "avalée" par le rescue_from,
cette erreur doit héritée d'`Exception` plutôt que `StandardError`.

NB: il faudrait parvenir à écrire un test pour vérifier ce comportement.

Pour être vérifié en dev, il faut activer `raise_delivery_errors`
comme en production.

app/lib/mail_delivery_error.rb

@@ -0,0 +1,6 @@
+# Inherit from `Exception` instead of `StandardError`
+# because this error is raised in a `rescue_from StandardError`,
+# so it would be shallowed otherwise.
+#
+# TODO: add a test which verify that the error will permit the job to retry
+class MailDeliveryError < Exception; end # rubocop:disable Lint/InheritException

app/mailers/concerns/mailer_monitoring_concern.rb

@@ -4,6 +4,10 @@ module MailerMonitoringConcern
   included do
     before_action :add_dolist_header
 
+    # Intercept & log any error, then re-raise so job will retry.
+    # NOTE: rescue_from order matters, later matchers are tried first.
+    rescue_from StandardError, with: :log_and_raise_delivery_error
+
     # Don’t retry to send a message if the server rejects the recipient address
     rescue_from Net::SMTPSyntaxError do |_exception|
       message.perform_deliveries = false
@@ -13,12 +17,10 @@ module MailerMonitoringConcern
       if /unexpected recipients/.match?(exception.message)
         message.perform_deliveries = false
       else
-        log_delivery_error(exception)
+        log_and_raise_delivery_error(exception)
       end
     end
 
-    rescue_from StandardError, with: :log_delivery_error
-
     # mandatory for dolist
     # used for tracking in Dolist UI
     # the delivery_method is yet unknown (:balancer)
@@ -27,13 +29,12 @@ module MailerMonitoringConcern
       headers['X-Dolist-Message-Name'] = action_name
     end
 
-    protected
-
-    def log_delivery_error(exception)
+    def log_and_raise_delivery_error(exception)
       EmailEvent.create_from_message!(message, status: "dispatch_error")
       Sentry.capture_exception(exception, extra: { to: message.to, subject: message.subject })
 
-      # TODO find a way to re attempt the job
+      # re-raise another error so job will retry later
+      raise MailDeliveryError.new(exception)
     end
   end
 end

spec/mailers/application_mailer_spec.rb

@@ -41,8 +41,8 @@ RSpec.describe ApplicationMailer, type: :mailer do
       expect(event.status).to eq('dispatched')
     end
 
-    context "when email is not sent" do
-      subject(:send_email) { UserMailer.ask_for_merge(user1, user2.email).deliver_now }
+    context "when there is an error and email are not sent" do
+      subject { UserMailer.ask_for_merge(user1, user2.email) }
 
       before do
         allow_any_instance_of(Mail::Message)
@@ -53,8 +53,8 @@ RSpec.describe ApplicationMailer, type: :mailer do
       context "smtp server busy" do
         let(:smtp_error) { Net::SMTPServerBusy.new }
 
-        it "creates an event" do
-          expect { send_email }.to change { EmailEvent.count }.by(1)
+        it "re-raise an error and creates an event" do
+          expect { subject.deliver_now }.to change { EmailEvent.count }.by(1).and raise_error(MailDeliveryError)
           expect(EmailEvent.last.status).to eq('dispatch_error')
         end
       end
@@ -62,8 +62,8 @@ RSpec.describe ApplicationMailer, type: :mailer do
       context "generic unknown error" do
         let(:smtp_error) { Net::OpenTimeout.new }
 
-        it "creates an event" do
-          expect { send_email }.to change { EmailEvent.count }.by(1)
+        it "re-raise an error and creates an event" do
+          expect { subject.deliver_now }.to change { EmailEvent.count }.by(1).and raise_error(MailDeliveryError)
           expect(EmailEvent.last.status).to eq('dispatch_error')
         end
       end

spec/mailers/invite_mailer_spec.rb

@@ -18,7 +18,13 @@ RSpec.describe InviteMailer, type: :mailer do
            v = send_mail_values.shift
            v == :raise ? raise("boom") : v
          end
-         mailer.body rescue nil
+
+         begin
+           mailer.body
+         rescue MailDeliveryError
+           nil
+         end
+
          mailer.body
          expect(TargetedUserLink.where(target_model: invite, user: invite.user).count).to eq(1)
        end
@@ -42,7 +48,13 @@ RSpec.describe InviteMailer, type: :mailer do
            v = send_mail_values.shift
            v == :raise ? raise("boom") : v
          end
-         mailer.body rescue nil
+
+         begin
+           mailer.body
+         rescue MailDeliveryError
+           nil
+         end
+
          mailer.body
          expect(TargetedUserLink.where(target_model: invite, user: invite.user).count).to eq(1)
        end