amelioration(email_event): re-lever une erreur dans un rescue_from ne la fait pas remonter. change de stratégie pour savoir si oui ou non un mail a ete envoye avec success.

app/lib/mail_delivery_error.rb

@@ -1,12 +0,0 @@
-# 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 # rubocop:disable Lint/InheritException
-  def initialize(original_exception)
-    super(original_exception.message)
-
-    set_backtrace(original_exception.backtrace)
-  end
-end

app/mailers/concerns/mailer_monitoring_concern.rb

@@ -2,10 +2,6 @@ module MailerMonitoringConcern
   extend ActiveSupport::Concern
 
   included do
-    # 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
@@ -14,21 +10,11 @@ module MailerMonitoringConcern
     rescue_from Net::SMTPServerBusy do |exception|
       if /unexpected recipients/.match?(exception.message)
         message.perform_deliveries = false
-      else
-        log_and_raise_delivery_error(exception)
       end
     end
 
     rescue_from Dolist::IgnorableError, with: :log_delivery_error
 
-    def log_and_raise_delivery_error(exception)
-      log_delivery_error(exception)
-      Sentry.capture_exception(exception, extra: { to: message.to, subject: message.subject })
-
-      # re-raise another error so job will retry later
-      raise MailDeliveryError.new(exception)
-    end
-
     def log_delivery_error(exception)
       EmailEvent.create_from_message!(message, status: "dispatch_error")
     end

app/models/email_event.rb

@@ -16,6 +16,7 @@ class EmailEvent < ApplicationRecord
   RETENTION_DURATION = 1.month
 
   enum status: {
+    pending: 'pending',
     dispatched: 'dispatched',
     dispatch_error: 'dispatch_error'
   }

app/services/email_delivering_interceptor.rb

@@ -0,0 +1,5 @@
+class EmailDeliveringInterceptor
+  def self.delivering_email(message)
+    EmailEvent.create_from_message!(message, status: "pending")
+  end
+end

config/initializers/mail_observers.rb

@@ -2,7 +2,9 @@
 # otherwise the observer won't be invoked.
 #
 require_relative "../../app/services/email_delivery_observer"
+require_relative "../../app/services/email_delivering_interceptor"
 
 ActiveSupport.on_load(:action_mailer) do |mailer|
+  mailer.register_interceptor EmailDeliveringInterceptor
   mailer.register_observer EmailDeliveryObserver
 end

spec/mailers/application_mailer_spec.rb

@@ -37,8 +37,8 @@ RSpec.describe ApplicationMailer, type: :mailer do
       end
 
       it 'raise classic error to retry' do
-        expect { subject }.to raise_error(MailDeliveryError)
+        expect { subject }.to raise_error(RuntimeError)
-        expect(EmailEvent.dolist_api.dispatch_error.count).to eq(1)
+        expect(EmailEvent.pending.count).to eq(1)
       end
     end
 
@@ -76,8 +76,9 @@ RSpec.describe ApplicationMailer, type: :mailer do
     let(:user2) { create(:user, email: "your@email.com") }
 
     it 'creates a new EmailEvent record with the correct information' do
-      expect { UserMailer.ask_for_merge(user1, user2.email).deliver_now }.to change { EmailEvent.count }.by(1)
+      expect { UserMailer.ask_for_merge(user1, user2.email).deliver_now }.to change { EmailEvent.count }.by(2)
       event = EmailEvent.last
+      expect(EmailEvent.first.status).to eq('pending')
 
       expect(event.to).to eq("your@email.com")
       expect(event.method).to eq("test")
@@ -85,31 +86,36 @@ RSpec.describe ApplicationMailer, type: :mailer do
       expect(event.processed_at).to be_within(1.second).of(Time.zone.now)
       expect(event.status).to eq('dispatched')
     end
+  end
+
+  context 'EmailDeliveringInterceptor is invoked' do
+    let(:user1) { create(:user) }
+    let(:user2) { create(:user, email: "your@email.com") }
 
     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)
-          .to receive(:deliver)
+          .to receive(:do_delivery)
           .and_raise(smtp_error)
       end
 
       context "smtp server busy" do
         let(:smtp_error) { Net::SMTPServerBusy.new('451 4.7.500 Server busy') }
 
-        it "re-raise an error and creates an event" do
+        it "catches the smtp error" do
-          expect { subject.deliver_now }.to change { EmailEvent.count }.by(1).and raise_error(MailDeliveryError)
+          expect { subject.deliver_now }.not_to raise_error
-          expect(EmailEvent.last.status).to eq('dispatch_error')
+          expect(EmailEvent.pending.count).to eq(1)
         end
       end
 
-      context "generic unknown error" do
+      context "does not catches other error" do
         let(:smtp_error) { Net::OpenTimeout.new }
 
         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 { subject.deliver_now }.to raise_error(Net::OpenTimeout)
-          expect(EmailEvent.last.status).to eq('dispatch_error')
+          expect(EmailEvent.pending.count).to eq(1)
         end
       end
     end

spec/mailers/invite_mailer_spec.rb

@@ -21,7 +21,7 @@ RSpec.describe InviteMailer, type: :mailer do
 
          begin
            mailer.body
-         rescue MailDeliveryError
+         rescue => e
            nil
          end
 
@@ -70,7 +70,7 @@ RSpec.describe InviteMailer, type: :mailer do
 
          begin
            mailer.body
-         rescue MailDeliveryError
+         rescue => e
            nil
          end