Discard pooled SMTP connections on terminal SMTP errors#19
Closed
MaximilianKohler wants to merge 2 commits into
Closed
Discard pooled SMTP connections on terminal SMTP errors#19MaximilianKohler wants to merge 2 commits into
MaximilianKohler wants to merge 2 commits into
Conversation
Some SMTP servers close a session after policy limits (e.g. time or message count) and return 421 Service not available, closing transmission channel, or close the connection mid-DATA. In these cases the SMTP connection is no longer usable and must not be returned to the pool. This change improves RFC-5321 compliance by treating 421 and connection-level errors as terminal, ensuring dead connections are discarded and retried on fresh connections.
GPT suggested this as well, but I didn't include it:
## 3️⃣ Retry-path behavior (acceptable, but tighten wording)
Your retry path is:
```go
retry, err := c.send(e)
...
_ = p.returnConn(c, err)
if !retry {
return err
}
```
This is fine **because**:
* Terminal errors discard the connection
* Retries will borrow a new connection
* Retry count is bounded
However, maintainers may ask:
> “What if `send()` returns retryable=true AND terminal=true?”
Answer:
That’s correct behavior — retry the message, but on a new connection.
To pre-empt that question, add **one comment** in `Send()`:
```go
// returnConn will discard the connection on terminal errors even if the
// message itself is retryable.
```
This prevents review churn.
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.
Some SMTP servers close a session after policy limits (e.g. time or message count) and return 421 Service not available, closing transmission channel, or close the connection mid-DATA.
In these cases the SMTP connection is no longer usable and must not be returned to the pool.
This change improves RFC-5321 compliance by treating 421 and connection-level errors as terminal, ensuring dead connections are discarded and retried on fresh connections.
This PR was created with ChatGPT and closes: #18