From c4a899656259071299e8068d5e2e2712506ed4c4 Mon Sep 17 00:00:00 2001
From: Mike Dalessio <mike@37signals.com>
Date: Thu, 4 Dec 2025 10:07:20 -0500
Subject: [PATCH] Retry mailer jobs on common networking and SMTP errors

This concern is lifted nearly verbatim from Basecamp.

ref: https://app.fizzy.do/5986089/cards/3300
---
 .../concerns/smtp_delivery_error_handling.rb  | 36 +++++++++++
 app/jobs/notification/bundle/deliver_job.rb   |  2 +
 .../action_mailer_mail_delivery_job.rb        |  3 +
 test/mailers/smtp_delivery_error_test.rb      | 59 +++++++++++++++++++
 4 files changed, 100 insertions(+)
 create mode 100644 app/jobs/concerns/smtp_delivery_error_handling.rb
 create mode 100644 lib/rails_ext/action_mailer_mail_delivery_job.rb
 create mode 100644 test/mailers/smtp_delivery_error_test.rb

diff --git a/app/jobs/concerns/smtp_delivery_error_handling.rb b/app/jobs/concerns/smtp_delivery_error_handling.rb
new file mode 100644
index 000000000..988fbf9d0
--- /dev/null
+++ b/app/jobs/concerns/smtp_delivery_error_handling.rb
@@ -0,0 +1,36 @@
+module SmtpDeliveryErrorHandling
+  extend ActiveSupport::Concern
+
+  included do
+    # Retry delivery to possibly-unavailable remote mailservers.
+    retry_on Net::OpenTimeout, Net::ReadTimeout, Socket::ResolutionError, wait: :polynomially_longer
+
+    # Net::SMTPServerBusy is SMTP error code 4xx, a temporary error.
+    # Common one we've seen is 452 4.3.1 Insufficient system storage.
+    # Patiently retry.
+    retry_on Net::SMTPServerBusy, wait: :polynomially_longer
+
+    # SMTP error 50x.
+    rescue_from Net::SMTPSyntaxError do |error|
+      case error.message
+      when /\A501 5\.1\.3/
+        # Ignore undeliverable email addresses.
+        Sentry.capture_exception error, level: :info if Fizzy.saas?
+      else
+        raise
+      end
+    end
+
+    # SMTP error 5xx except 50x and 53x.
+    # * 550 5.1.1: Unknown users
+    # * 552 5.6.0: Message/headers too large
+    rescue_from Net::SMTPFatalError do |error|
+      case error.message
+      when /\A550 5\.1\.1/, /\A552 5\.6\.0/, /\A555 5\.5\.4/
+        Sentry.capture_exception error, level: :info if Fizzy.saas?
+      else
+        raise
+      end
+    end
+  end
+end
diff --git a/app/jobs/notification/bundle/deliver_job.rb b/app/jobs/notification/bundle/deliver_job.rb
index 40e1df479..11f6bcda5 100644
--- a/app/jobs/notification/bundle/deliver_job.rb
+++ b/app/jobs/notification/bundle/deliver_job.rb
@@ -1,4 +1,6 @@
 class Notification::Bundle::DeliverJob < ApplicationJob
+  include SmtpDeliveryErrorHandling
+
   queue_as :backend
 
   def perform(bundle)
diff --git a/lib/rails_ext/action_mailer_mail_delivery_job.rb b/lib/rails_ext/action_mailer_mail_delivery_job.rb
new file mode 100644
index 000000000..9da732dda
--- /dev/null
+++ b/lib/rails_ext/action_mailer_mail_delivery_job.rb
@@ -0,0 +1,3 @@
+Rails.application.config.to_prepare do
+  ActionMailer::MailDeliveryJob.include SmtpDeliveryErrorHandling
+end
diff --git a/test/mailers/smtp_delivery_error_test.rb b/test/mailers/smtp_delivery_error_test.rb
new file mode 100644
index 000000000..5ba764585
--- /dev/null
+++ b/test/mailers/smtp_delivery_error_test.rb
@@ -0,0 +1,59 @@
+require "test_helper"
+
+class SmtpDeliveryErrorTest < ActionMailer::TestCase
+  class TestMailer < ApplicationMailer
+    def smtp_syntax_error(message)
+      raise Net::SMTPSyntaxError, Net::SMTP::Response.parse(message)
+    end
+
+    def smtp_fatal_error(message)
+      raise Net::SMTPFatalError, Net::SMTP::Response.parse(message)
+    end
+
+    def ephemeral_retry
+      self.class.goes_boom_once
+    end
+
+    def self.goes_boom_once
+      # Stubbed in test to raise exception once
+    end
+  end
+  tests TestMailer
+
+  test "deliver_later ignores bad recipient addresses" do
+    assert_nothing_raised do
+      perform_enqueued_jobs only: ActionMailer::MailDeliveryJob do
+        TestMailer.smtp_syntax_error("501 5.1.3 Bad recipient address syntax\n").deliver_later
+      end
+    end
+  end
+
+  test "deliver_later ignores rejected recipient addresses" do
+    assert_nothing_raised do
+      perform_enqueued_jobs only: ActionMailer::MailDeliveryJob do
+        TestMailer.smtp_fatal_error("550 5.1.1 fooaddress: Recipient address rejected: User unknown in local recipient table\n").deliver_later
+      end
+    end
+  end
+
+  test "deliver_later re-raises other SMTP syntax errors" do
+    perform_enqueued_jobs only: ActionMailer::MailDeliveryJob do
+      assert_raises Net::SMTPSyntaxError do
+        TestMailer.smtp_syntax_error("not a recipient address error").deliver_later
+      end
+    end
+  end
+
+
+  [ Net::OpenTimeout, Net::ReadTimeout, Net::SMTPServerBusy.new(Net::SMTP::Response.parse("4xx Server Busy")) ].each do |exception|
+    test "deliver_later retries temporary #{exception}" do
+      TestMailer.stubs(:goes_boom_once).raises(exception).then.returns(nil)
+
+      perform_enqueued_jobs only: ActionMailer::MailDeliveryJob do
+        assert_nothing_raised do
+          TestMailer.ephemeral_retry.deliver_later
+        end
+      end
+    end
+  end
+end