From cc68bc895267899f4c301c027505a3484ddbcab8 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Tue, 18 Aug 2026 22:27:06 +0100 Subject: [PATCH] Terminate email send on context cancellation Context was checked once before the send, and the sender applied its timeout to the connection setup only, so a server accepting the connection and stalling afterwards blocked Send with no way out. SendContext of go-pkgz/email covers the whole SMTP transaction. Requires go-pkgz/email with SendContext, go.mod bump is not included as that release is not out yet. --- email.go | 14 ++++++----- email_test.go | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/email.go b/email.go index 4bc2efa..80930be 100644 --- a/email.go +++ b/email.go @@ -2,6 +2,7 @@ package notify import ( "context" + "errors" "fmt" "net/mail" "net/url" @@ -23,7 +24,7 @@ type SMTPParams struct { LoginAuth bool // LOGIN auth method instead of default PLAIN, needed for Office 365 and outlook.com Username string // username Password string // password - TimeOut time.Duration // TCP connection timeout + TimeOut time.Duration // TCP connection timeout, the rest of the transaction is bound by the context of Send } // Email notifications client @@ -95,12 +96,13 @@ func (e *Email) Send(ctx context.Context, destination, text string) error { return fmt.Errorf("problem parsing destination: %w", err) } - select { - case <-ctx.Done(): - return ctx.Err() - default: - return e.sender.Send(text, emailParams) + // SendContext terminates the transaction when ctx is done, including the parts after the connection is made + err = e.sender.SendContext(ctx, text, emailParams) + if err != nil && ctx.Err() != nil && !errors.Is(err, ctx.Err()) { + // transaction was interrupted, report why on top of the error it failed with + return fmt.Errorf("%w: %w", ctx.Err(), err) } + return err } // Schema returns schema prefix supported by this client diff --git a/email_test.go b/email_test.go index 24f1a8a..53cdc65 100644 --- a/email_test.go +++ b/email_test.go @@ -2,6 +2,11 @@ package notify import ( "context" + "fmt" + "io" + "net" + "strconv" + "sync" "testing" "time" @@ -74,3 +79,64 @@ func TestEmailSendClientError(t *testing.T) { cancel() assert.EqualError(t, email.Send(ctx, "mailto:test@example.org", ""), "context canceled") } + +func TestEmail_SendCancellationAfterConnect(t *testing.T) { + // server accepts the connection, greets and stops responding + ln, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + + var wg sync.WaitGroup + defer wg.Wait() + defer func() { _ = ln.Close() }() + + var once sync.Once + greeted := make(chan struct{}) // closed once the client sent a command, i.e. it is past the connection and the greeting + wg.Go(func() { + for { + conn, e := ln.Accept() + if e != nil { + return + } + wg.Go(func() { + defer func() { _ = conn.Close() }() + // server side gives up on its own, so a regression fails the test instead of hanging the suite + _ = conn.SetDeadline(time.Now().Add(time.Second * 20)) + _, _ = fmt.Fprint(conn, "220 localhost ESMTP stub\r\n") + buf := make([]byte, 1) + if _, e := conn.Read(buf); e == nil { + once.Do(func() { close(greeted) }) + } + _, _ = io.Copy(io.Discard, conn) + }) + } + }) + + host, portStr, err := net.SplitHostPort(ln.Addr().String()) + require.NoError(t, err) + port, err := strconv.Atoi(portStr) + require.NoError(t, err) + + // connection timeout is long on purpose, the context should terminate the send + email := NewEmail(SMTPParams{Host: host, Port: port, TimeOut: time.Minute}) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // cancel only once the transaction is past the connection, otherwise the test proves nothing + go func() { + <-greeted + cancel() + }() + failsafe := time.AfterFunc(time.Second*30, cancel) + defer failsafe.Stop() + + st := time.Now() + err = email.Send(ctx, "mailto:test@example.org?from=notify@example.org", "test") + require.Error(t, err) + require.ErrorIs(t, err, context.Canceled, "cancellation is visible to the caller") + assert.Less(t, time.Since(st), time.Second*5, "send is terminated by the context, not by the stalled server") + select { + case <-greeted: + default: + t.Fatal("send was canceled before the connection was established, the test proves nothing") + } +}