Retry transient (4xx) SMTP replies, including on the DATA terminator#20
Open
josephsellers wants to merge 1 commit into
Open
Retry transient (4xx) SMTP replies, including on the DATA terminator#20josephsellers wants to merge 1 commit into
josephsellers wants to merge 1 commit into
Conversation
canRetry() only classified connection-level errors (net.Error, net.OpError,
io.EOF) as retriable, and conn.send() hardcoded `return false` for the error
from w.Close() -- the reply to the DATA terminator ("."). As a result, a
transient 4xx reply at end-of-DATA was never retried even with
MaxMessageRetries set, and surfaced to the caller as a hard failure.
This is common with AWS SES, which returns "451 4.4.2 Timeout waiting for
data from client." on a pooled connection whose server-side state has gone
stale; an immediate retry on a fresh connection succeeds.
- canRetry(): also treat a 4xx *textproto.Error as retriable (5xx stays
permanent, so bad-recipient etc. is not retried).
- send()/w.Close(): retry on a 4xx reply, but NOT on a connection-level
error there -- after the terminator the server may have accepted the
message before the socket dropped, so retrying could duplicate delivery.
- returnConn(): generalise the 421-closes-connection rule (knadh#19) to all 4xx,
so a retry dials a fresh connection rather than reusing the poisoned one.
Adds TestCanRetry covering 4xx/5xx/connection/other classification.
[Used Claude Code 🤖]
josephsellers
added a commit
to josephsellers/libredesk
that referenced
this pull request
Jul 15, 2026
Outbound sends via AWS SES intermittently failed with "451 4.4.2 Timeout waiting for data from client." on stale pooled connections. smtppool v1.x marks all SMTP-conversation errors (including the 4xx reply at the DATA terminator) as non-retriable, so MaxMessageRetries never fired and the failure surfaced to the agent, who had to click Retry manually. Point the dependency at josephsellers/smtppool v1.3.1-libredesk, which backports the retry-classification fix (transient 4xx -> retry on a fresh connection; 5xx stays permanent; duplicate-safe at the DATA terminator). Temporary: upstream PR is knadh/smtppool#20 (against the v2 module line). Drop this replace once we adopt the released, fixed smtppool. [Used Claude Code 🤖]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
While using this library (via LibreDesk, which relies on SES) I hit intermittent hard send failures that succeeded immediately on a manual retry. The message was:
AWS SES returns this transient reply when a pooled connection's server-side state has gone stale. A retry on a fresh connection always succeeds, but
MaxMessageRetriesnever kicked in for it.Cause
Two gaps meant the transient reply was classified as non-retriable:
canRetry()only recognised connection-level errors (net.Error,net.OpError,io.EOF). A transient SMTP reply is a*textproto.Error, socanRetry(451)returnedfalse.conn.send(), the error fromw.Close()(the reply to the DATA terminator., which is exactly where SES returns the 451) was hardcoded toreturn false, err, bypassingcanRetryentirely.Change
canRetry()now also treats a 4xx*textproto.Erroras retriable. 5xx stays permanent, so a rejected recipient / oversized message is still not retried.send()/w.Close()now retries on a 4xx reply, but deliberately not on a connection-level error at that point: after the terminator the server may have accepted the message before the socket dropped, so retrying a raw connection error there could cause duplicate delivery. A 4xx reply is an explicit non-acceptance, so retrying it is safe.returnConn()generalises the existing421-closes-connection rule (Discard pooled SMTP connections on terminal SMTP errors #19) to all 4xx, so a retry dials a fresh connection instead of reusing the compromised one.Test
Adds
TestCanRetrycovering the classification (451/421/450 retriable; 550/552 not;io.EOF/net.OpErrorretriable; plain error / nil not). It needs no SMTP server. The existing MailHog-backed tests are unchanged.[Used Claude Code 🤖]