From 647e39b2787605bbf9658e58fa4c457191a3c660 Mon Sep 17 00:00:00 2001
From: Christoph Geschwind <christoph.geschwind@mixxt.de>
Date: Wed, 30 Aug 2023 13:09:04 +0200
Subject: [PATCH] Log delivery errors in ActionMailer::LogSubscriber

In case of mail delivery errors, the old ActionMailer::LogSubscriber
erroneously logged a "Delivered mail ..." line.
With this change it checks the passed exception object and logs a
"Failed delivery of mail" line instead.

Co-authored-by: Christian Esser <c_esser@yahoo.de>
---
 .../lib/action_mailer/log_subscriber.rb       |  5 +++--
 actionmailer/test/log_subscriber_test.rb      | 22 +++++++++++++++++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/actionmailer/lib/action_mailer/log_subscriber.rb b/actionmailer/lib/action_mailer/log_subscriber.rb
index 301f834c05..130d5d83e6 100644
--- a/actionmailer/lib/action_mailer/log_subscriber.rb
+++ b/actionmailer/lib/action_mailer/log_subscriber.rb
@@ -11,8 +11,9 @@ class LogSubscriber < ActiveSupport::LogSubscriber
     # An email was delivered.
     def deliver(event)
       info do
-        perform_deliveries = event.payload[:perform_deliveries]
-        if perform_deliveries
+        if exception = event.payload[:exception_object]
+          "Failed delivery of mail #{event.payload[:message_id]} error_class=#{exception.class} error_message=#{exception.message.inspect}"
+        elsif event.payload[:perform_deliveries]
           "Delivered mail #{event.payload[:message_id]} (#{event.duration.round(1)}ms)"
         else
           "Skipped delivery of mail #{event.payload[:message_id]} as `perform_deliveries` is false"
diff --git a/actionmailer/test/log_subscriber_test.rb b/actionmailer/test/log_subscriber_test.rb
index a36f44bae5..d473742d0b 100644
--- a/actionmailer/test/log_subscriber_test.rb
+++ b/actionmailer/test/log_subscriber_test.rb
@@ -19,6 +19,15 @@ def receive(mail)
     end
   end
 
+  class BogusDelivery
+    def initialize(*)
+    end
+
+    def deliver!(mail)
+      raise "failed"
+    end
+  end
+
   def set_logger(logger)
     ActionMailer::Base.logger = logger
   end
@@ -50,4 +59,17 @@ def test_deliver_message_when_perform_deliveries_is_false
   ensure
     BaseMailer.deliveries.clear
   end
+
+  def test_deliver_message_when_exception_happened
+    previous_delivery_method = BaseMailer.delivery_method
+    BaseMailer.delivery_method = BogusDelivery
+
+    assert_raises(RuntimeError) { BaseMailer.welcome(message_id: "123@abc").deliver_now }
+    wait
+
+    assert_equal(1, @logger.logged(:info).size)
+    assert_equal('Failed delivery of mail 123@abc error_class=RuntimeError error_message="failed"', @logger.logged(:info).first)
+  ensure
+    BaseMailer.delivery_method = previous_delivery_method
+  end
 end