Skip to content

Commit c4a8996

Browse files
committed
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
1 parent 2fcdd92 commit c4a8996

File tree

4 files changed

+100
-0
lines changed

4 files changed

+100
-0
lines changed
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
module SmtpDeliveryErrorHandling
2+
extend ActiveSupport::Concern
3+
4+
included do
5+
# Retry delivery to possibly-unavailable remote mailservers.
6+
retry_on Net::OpenTimeout, Net::ReadTimeout, Socket::ResolutionError, wait: :polynomially_longer
7+
8+
# Net::SMTPServerBusy is SMTP error code 4xx, a temporary error.
9+
# Common one we've seen is 452 4.3.1 Insufficient system storage.
10+
# Patiently retry.
11+
retry_on Net::SMTPServerBusy, wait: :polynomially_longer
12+
13+
# SMTP error 50x.
14+
rescue_from Net::SMTPSyntaxError do |error|
15+
case error.message
16+
when /\A501 5\.1\.3/
17+
# Ignore undeliverable email addresses.
18+
Sentry.capture_exception error, level: :info if Fizzy.saas?
19+
else
20+
raise
21+
end
22+
end
23+
24+
# SMTP error 5xx except 50x and 53x.
25+
# * 550 5.1.1: Unknown users
26+
# * 552 5.6.0: Message/headers too large
27+
rescue_from Net::SMTPFatalError do |error|
28+
case error.message
29+
when /\A550 5\.1\.1/, /\A552 5\.6\.0/, /\A555 5\.5\.4/
30+
Sentry.capture_exception error, level: :info if Fizzy.saas?
31+
else
32+
raise
33+
end
34+
end
35+
end
36+
end

app/jobs/notification/bundle/deliver_job.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
class Notification::Bundle::DeliverJob < ApplicationJob
2+
include SmtpDeliveryErrorHandling
3+
24
queue_as :backend
35

46
def perform(bundle)
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Rails.application.config.to_prepare do
2+
ActionMailer::MailDeliveryJob.include SmtpDeliveryErrorHandling
3+
end
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
require "test_helper"
2+
3+
class SmtpDeliveryErrorTest < ActionMailer::TestCase
4+
class TestMailer < ApplicationMailer
5+
def smtp_syntax_error(message)
6+
raise Net::SMTPSyntaxError, Net::SMTP::Response.parse(message)
7+
end
8+
9+
def smtp_fatal_error(message)
10+
raise Net::SMTPFatalError, Net::SMTP::Response.parse(message)
11+
end
12+
13+
def ephemeral_retry
14+
self.class.goes_boom_once
15+
end
16+
17+
def self.goes_boom_once
18+
# Stubbed in test to raise exception once
19+
end
20+
end
21+
tests TestMailer
22+
23+
test "deliver_later ignores bad recipient addresses" do
24+
assert_nothing_raised do
25+
perform_enqueued_jobs only: ActionMailer::MailDeliveryJob do
26+
TestMailer.smtp_syntax_error("501 5.1.3 Bad recipient address syntax\n").deliver_later
27+
end
28+
end
29+
end
30+
31+
test "deliver_later ignores rejected recipient addresses" do
32+
assert_nothing_raised do
33+
perform_enqueued_jobs only: ActionMailer::MailDeliveryJob do
34+
TestMailer.smtp_fatal_error("550 5.1.1 fooaddress: Recipient address rejected: User unknown in local recipient table\n").deliver_later
35+
end
36+
end
37+
end
38+
39+
test "deliver_later re-raises other SMTP syntax errors" do
40+
perform_enqueued_jobs only: ActionMailer::MailDeliveryJob do
41+
assert_raises Net::SMTPSyntaxError do
42+
TestMailer.smtp_syntax_error("not a recipient address error").deliver_later
43+
end
44+
end
45+
end
46+
47+
48+
[ Net::OpenTimeout, Net::ReadTimeout, Net::SMTPServerBusy.new(Net::SMTP::Response.parse("4xx Server Busy")) ].each do |exception|
49+
test "deliver_later retries temporary #{exception}" do
50+
TestMailer.stubs(:goes_boom_once).raises(exception).then.returns(nil)
51+
52+
perform_enqueued_jobs only: ActionMailer::MailDeliveryJob do
53+
assert_nothing_raised do
54+
TestMailer.ephemeral_retry.deliver_later
55+
end
56+
end
57+
end
58+
end
59+
end

0 commit comments

Comments
 (0)