You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(mailer): time out when the SMTP server never sends its greeting - #2043
An email send hangs forever if the SMTP server accepts the connection but never sends its greeting. This sets a deadline as soon as the connection opens, so the send fails after the dialer timeout (10s by default).
How it happens: a mail server (or a proxy or firewall in front of it) accepts the TCP connection and then stays silent. gopkg.in/mail.v2Dialer.Dial reads the greeting (smtp.go:91, inside smtpNewClient) before it sets the connection deadline (smtp.go:96-98), so that read has no deadline. Every mail path goes through DialerImpl.DialAndSend: login OTP, magic link, invitations, PAT alerts, org and user delete emails. Most run inside a request, so the request hangs too. It needs a silent server, so it is rare.
Changes
pkg/mailer/dialer.go: wrap mail.NetDialTimeout once (sync.Once, from NewDialerImpl) so each new connection gets a deadline of the dialer Timeout right away. The library then resets the deadline after the greeting as before.
pkg/mailer/dialer_test.go: new test.
Technical Details
Cause: the library sets the deadline after reading the greeting, not before. Why this way: the issue lists two options. A wrapper on the library's own NetDialTimeout hook is a few lines. Moving to another SMTP library is a much larger change. Scope:NetDialTimeout is a package-level variable, so the wrapper applies to every mail.v2 dialer in the process. pkg/mailer is the only place in the repo that creates one (grep NewDialer). The wrapper only adds a deadline of the dialer's own timeout, and does nothing if the timeout is 0.
Test Plan
Build and type checking passes
Manual testing completed (not done against a real mail server)
Test TestDialerImpl_DialAndSend_SilentServer starts a local TCP listener on 127.0.0.1 that accepts and never writes.
Before the fix: FAIL ... DialAndSend still blocked after 3s: no deadline on the greeting read
After the fix: ok github.com/raystack/frontier/pkg/mailer 0.540s
Commands: go vet ./pkg/mailer/..., go test ./pkg/mailer/..., gofmt -l pkg/mailer. The full suite and integration tests were not run.
Risk: low. A server that is slow but not silent now has the dialer timeout to send its greeting, which is the timeout already meant for the whole connection setup.
Mail delivery now respects the configured timeout when connecting to an unresponsive mail server, preventing sends from remaining blocked indefinitely.
Connection setup failures are returned promptly, so mail operations can report errors instead of waiting without a response.
The mail dialer now sets a deadline immediately after a successful connection when the configured timeout is positive. A test checks that sending to a server that accepts connections but sends no greeting returns an error.
Changes: SMTP greeting timeout
Layer / File(s)
Summary
Install and verify greeting deadline pkg/mailer/dialer.go, pkg/mailer/dialer_test.go
NewDialerImpl installs a wrapper around mail.NetDialTimeout. The wrapper sets a deadline on successful connections when the timeout is positive and closes the connection if setting the deadline fails. The test checks that DialAndSend returns an error when the server does not send a greeting.
raystack/frontier#2034: Both changes use the shared mail send path. This PR makes a send to a silent SMTP server return an error rather than block.
Suggested reviewers:rohilsurana
Merge Risk: Low · up to 75d05
The change prevents indefinite waits for silent SMTP servers. It is mergeable with a bounded test improvement: verify that failure follows the configured timeout rather than merely occurring within three seconds.
Pre-merge checks: 2
Check name
Status
Explanation
Linked Issues check
[#2039] The only direct linked issue requires that the SMTP greeting read not block indefinitely when the server accepts a connection and stays silent. At head, pkg/mailer/dialer.go adds `installGre…
Out of Scope Changes check
The whole-PR diff against the merge base eaa557a5 changes two files. pkg/mailer/dialer.go adds the one-time NetDialTimeout wrapper and calls it from NewDialerImpl. pkg/mailer/dialer_test.go …
Full details: Linked Issues check
Explanation
[#2039] The only direct linked issue requires that the SMTP greeting read not block indefinitely when the server accepts a connection and stays silent. At head, pkg/mailer/dialer.go adds installGreetingDeadline(), which wraps mail.NetDialTimeout once through sync.Once. NewDialerImpl calls it before mail.NewDialer. The wrapper calls the original dial, then sets conn.SetDeadline(time.Now().Add(timeout)) when timeout > 0. The greeting read therefore has a finite deadline before the library reads the greeting. This matches the fix proposed in the issue. pkg/mailer/dialer_test.go adds TestDialerImpl_DialAndSend_SilentServer. It uses a local listener that accepts and never writes, and it expects DialAndSend to return an error within 3s with a 200 ms dialer timeout. Limit of evidence: I could not read the gopkg.in/mail.v2 source. The repository has no vendor copy, so I could not confirm that the library passes d.Timeout as the timeout argument of the hook. The author reports the test failing before the change and passing after it (0.540s). I did not rerun the test. The requirement is met in the default configuration, where the wrapper bounds the greeting read to the library's finite dial timeout.
Full details: Out of Scope Changes check
Explanation
The whole-PR diff against the merge base eaa557a5 changes two files. pkg/mailer/dialer.go adds the one-time NetDialTimeout wrapper and calls it from NewDialerImpl. pkg/mailer/dialer_test.go is a new test for the silent-server case. Both changes implement the objective of #2039. The wrapper is process-wide because NetDialTimeout is a package variable. The PR description discloses this. The repository creates mail.v2 dialers only in pkg/mailer, so no unrelated dialer is affected. No unrelated files or behavior changes appear in the reviewed diff.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Assert the greeting timeout in the silent-server test.
The test allows up to three seconds and accepts any returned error. A greeting deadline near two seconds would still pass, even though the test comment says the send fails after the dialer timeout of 200 ms. Record the start time before the send. After the error, assert that the elapsed time stays under one second. This leaves margin for scheduling delay above the 200 ms timeout.
Suggested fix
done := make(chan error, 1)
+ start := time.Now()
go func() { done <- d.DialAndSend(msg) }()
select {
case err := <-done:
assert.Error(t, err)
+ assert.Less(t, time.Since(start), time.Second, "greeting read must end near the 200ms dialer timeout")
Reviewing files that changed from the base of the PR and between eaa557a and 75d05a4.
Files selected for processing: 2
pkg/mailer/dialer.go
pkg/mailer/dialer_test.go
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
This branch has not been deployed
No deployments
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
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.
Summary
An email send hangs forever if the SMTP server accepts the connection but never sends its greeting. This sets a deadline as soon as the connection opens, so the send fails after the dialer timeout (10s by default).
How it happens: a mail server (or a proxy or firewall in front of it) accepts the TCP connection and then stays silent.
gopkg.in/mail.v2Dialer.Dialreads the greeting (smtp.go:91, insidesmtpNewClient) before it sets the connection deadline (smtp.go:96-98), so that read has no deadline. Every mail path goes throughDialerImpl.DialAndSend: login OTP, magic link, invitations, PAT alerts, org and user delete emails. Most run inside a request, so the request hangs too. It needs a silent server, so it is rare.Changes
pkg/mailer/dialer.go: wrapmail.NetDialTimeoutonce (sync.Once, fromNewDialerImpl) so each new connection gets a deadline of the dialerTimeoutright away. The library then resets the deadline after the greeting as before.pkg/mailer/dialer_test.go: new test.Technical Details
Cause: the library sets the deadline after reading the greeting, not before.
Why this way: the issue lists two options. A wrapper on the library's own
NetDialTimeouthook is a few lines. Moving to another SMTP library is a much larger change.Scope:
NetDialTimeoutis a package-level variable, so the wrapper applies to everymail.v2dialer in the process.pkg/maileris the only place in the repo that creates one (grep NewDialer). The wrapper only adds a deadline of the dialer's own timeout, and does nothing if the timeout is 0.Test Plan
Test
TestDialerImpl_DialAndSend_SilentServerstarts a local TCP listener on 127.0.0.1 that accepts and never writes.FAIL ... DialAndSend still blocked after 3s: no deadline on the greeting readok github.com/raystack/frontier/pkg/mailer 0.540sgo vet ./pkg/mailer/...,go test ./pkg/mailer/...,gofmt -l pkg/mailer. The full suite and integration tests were not run.Risk: low. A server that is slow but not silent now has the dialer timeout to send its greeting, which is the timeout already meant for the whole connection setup.
Fixes #2039