From 8cc440a6dd594f6a149b71c70d3383f93445c4f0 Mon Sep 17 00:00:00 2001 From: Musa Date: Tue, 1 Sep 2026 23:30:41 +0100 Subject: [PATCH 1/4] notify: add GetFailureReasonFromSMTPCode for SMTP-based notifiers SMTP's 4xx/5xx reply codes are the inverse of HTTP's: 4xx is a transient failure (retry later), 5xx is permanent. 535 (RFC 4954) is broken out as an auth failure, mirroring how HTTP 401/403 map to AuthErrorReason. Signed-off-by: Musa --- notify/util.go | 21 +++++++++++++++++++++ notify/util_test.go | 20 ++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/notify/util.go b/notify/util.go index fe4c9ea508..f69b3b4970 100644 --- a/notify/util.go +++ b/notify/util.go @@ -343,3 +343,24 @@ func GetFailureReasonFromStatusCode(statusCode int) Reason { return DefaultReason } + +// GetFailureReasonFromSMTPCode returns the reason for the failure based on +// the SMTP reply code provided. Note that SMTP's 4xx/5xx split is the +// inverse of HTTP's: an SMTP 4xx is a transient failure (the server is +// asking the client to retry later), while a 5xx is permanent (the server +// is rejecting the request outright). This mirrors the retry semantics +// already used elsewhere in Alertmanager, not the HTTP status code ranges. +func GetFailureReasonFromSMTPCode(code int) Reason { + if code == 535 { + // RFC 4954: 535 is the standard reply for authentication failure. + return AuthErrorReason + } + if code/100 == 4 { + return ServerErrorReason + } + if code/100 == 5 { + return ClientErrorReason + } + + return DefaultReason +} diff --git a/notify/util_test.go b/notify/util_test.go index 2c2d4922e8..37c7336923 100644 --- a/notify/util_test.go +++ b/notify/util_test.go @@ -273,3 +273,23 @@ func TestGetFailureReasonFromStatusCode(t *testing.T) { }) } } + +func TestGetFailureReasonFromSMTPCode(t *testing.T) { + for _, tc := range []struct { + name string + code int + expected Reason + }{ + {"AuthenticationFailed", 535, AuthErrorReason}, + {"TransientMailboxUnavailable", 450, ServerErrorReason}, + {"ServiceNotAvailable", 421, ServerErrorReason}, + {"MailboxUnavailable", 550, ClientErrorReason}, + {"SyntaxError", 501, ClientErrorReason}, + {"Success", 250, DefaultReason}, + {"IntermediateReply", 354, DefaultReason}, + } { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expected, GetFailureReasonFromSMTPCode(tc.code)) + }) + } +} From 9641847373102d2788ec6939f5ae3788c36c470e Mon Sep 17 00:00:00 2001 From: Musa Date: Tue, 1 Sep 2026 23:30:41 +0100 Subject: [PATCH 2/4] notify/email: report failure reason from SMTP reply codes Applies GetFailureReasonFromSMTPCode at every point Notify can receive a *textproto.Error from the SMTP server (EHLO, STARTTLS, AUTH, MAIL, RCPT, DATA, and the final delivery response), so the email notifier's numTotalFailedNotifications reason label matches what the HTTP-based notifiers already report. Ref: prometheus/alertmanager#3231 Signed-off-by: Musa --- notify/email/email.go | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/notify/email/email.go b/notify/email/email.go index 6ffb99ba3f..5e314fe217 100644 --- a/notify/email/email.go +++ b/notify/email/email.go @@ -69,6 +69,19 @@ func New(c *config.EmailConfig, t *template.Template, l *slog.Logger) *Email { return &Email{conf: c, tmpl: t, logger: l, hostname: h} } +// wrapSMTPErr formats err with the given context message and, if err +// carries an SMTP reply code, wraps the result in a notify.ErrorWithReason +// so the failure surfaces in Alertmanager's per-reason notification metrics +// the same way HTTP-based notifiers already do. +func wrapSMTPErr(context string, err error) error { + wrapped := fmt.Errorf("%s: %w", context, err) + + if tpErr, ok := errors.AsType[*textproto.Error](err); ok { + return notify.NewErrorWithReason(notify.GetFailureReasonFromSMTPCode(tpErr.Code), wrapped) + } + return wrapped +} + // auth resolves a string of authentication mechanisms. func (n *Email) auth(mechs string) (smtp.Auth, error) { username := n.conf.AuthUsername @@ -177,7 +190,7 @@ func (n *Email) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { if n.conf.Hello != "" { err = c.Hello(n.conf.Hello) if err != nil { - return true, fmt.Errorf("send EHLO command: %w", err) + return true, wrapSMTPErr("send EHLO command", err) } } @@ -196,7 +209,7 @@ func (n *Email) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { } if err := c.StartTLS(tlsConf); err != nil { - return true, fmt.Errorf("send STARTTLS command: %w", err) + return true, wrapSMTPErr("send STARTTLS command", err) } } @@ -207,7 +220,7 @@ func (n *Email) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { } if auth != nil { if err := c.Auth(auth); err != nil { - return true, fmt.Errorf("%T auth: %w", auth, err) + return true, wrapSMTPErr(fmt.Sprintf("%T auth", auth), err) } } } @@ -234,7 +247,7 @@ func (n *Email) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { return false, fmt.Errorf("must be exactly one 'from' address (got: %d)", len(addrs)) } if err = c.Mail(addrs[0].Address); err != nil { - return true, fmt.Errorf("send MAIL command: %w", err) + return true, wrapSMTPErr("send MAIL command", err) } addrs, err = mail.ParseAddressList(to) if err != nil { @@ -242,14 +255,14 @@ func (n *Email) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { } for _, addr := range addrs { if err = c.Rcpt(addr.Address); err != nil { - return true, fmt.Errorf("send RCPT command: %w", err) + return true, wrapSMTPErr("send RCPT command", err) } } // Send the email headers and body. message, err := c.Data() if err != nil { - return true, fmt.Errorf("send DATA command: %w", err) + return true, wrapSMTPErr("send DATA command", err) } closeOnce := sync.OnceValue(func() error { return message.Close() @@ -375,7 +388,7 @@ func (n *Email) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { // Complete the message and await response. if err = closeOnce(); err != nil { - return true, fmt.Errorf("delivery failure: %w", err) + return true, wrapSMTPErr("delivery failure", err) } success = true From 61a67bdce75a6e7dd97526bd9f8c878bb5d6542c Mon Sep 17 00:00:00 2001 From: Musa Date: Tue, 1 Sep 2026 23:30:41 +0100 Subject: [PATCH 3/4] notify/email: assert failure reason in TestEmailRejected The mock server already rejects at DATA with a 501 (permanent); assert that now surfaces as ClientErrorReason. Signed-off-by: Musa --- notify/email/email_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/notify/email/email_test.go b/notify/email/email_test.go index 36d2bd0bbe..1df44ce0af 100644 --- a/notify/email/email_test.go +++ b/notify/email/email_test.go @@ -736,6 +736,14 @@ func TestEmailRejected(t *testing.T) { require.ErrorContains(t, err, "501") require.ErrorContains(t, err, "5.5.4") require.True(t, retry) + + // A 501 (5xx) SMTP reply is a permanent failure, which should surface + // as ClientErrorReason, mirroring how HTTP-based notifiers report 4xx + // responses (SMTP's 4xx/5xx split is the inverse of HTTP's). + var reasonErr *notify.ErrorWithReason + require.ErrorAs(t, err, &reasonErr, "expected error to carry a notify.ErrorWithReason") + require.Equal(t, notify.ClientErrorReason, reasonErr.Reason) + require.NoError(t, srv.Shutdown(ctx)) require.Eventuallyf(t, func() bool { From ee5ec907f09fa05d20c90a30cde48a86f17cec71 Mon Sep 17 00:00:00 2001 From: Musa Date: Tue, 15 Sep 2026 17:09:20 +0100 Subject: [PATCH 4/4] notify/email: wrap SMTP client creation error smtp.NewClient reads the server's initial greeting and returns a *textproto.Error for non-220 responses (e.g. 421 when the server is temporarily unavailable). Route that through wrapSMTPErr like every other SMTP stage, so a bad greeting gets its correct failure reason instead of falling back to DefaultReason. Adds TestEmailGreetingRejected covering a 421 greeting rejection. Signed-off-by: Musa --- notify/email/email.go | 2 +- notify/email/email_test.go | 50 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/notify/email/email.go b/notify/email/email.go index 5e314fe217..4b4cd6650a 100644 --- a/notify/email/email.go +++ b/notify/email/email.go @@ -178,7 +178,7 @@ func (n *Email) Notify(ctx context.Context, as ...*types.Alert) (bool, error) { c, err = smtp.NewClient(conn, n.conf.Smarthost.Host) if err != nil { conn.Close() - return true, fmt.Errorf("create SMTP client: %w", err) + return true, wrapSMTPErr("create SMTP client", err) } defer func() { // Try to clean up after ourselves but don't log anything if something has failed. diff --git a/notify/email/email_test.go b/notify/email/email_test.go index 1df44ce0af..e043a7887e 100644 --- a/notify/email/email_test.go +++ b/notify/email/email_test.go @@ -752,6 +752,56 @@ func TestEmailRejected(t *testing.T) { }, time.Second*10, time.Millisecond*100, "mock SMTP server goroutine failed to close in time") } +// TestEmailGreetingRejected simulates a server that rejects the connection at the initial SMTP +// greeting (before any session is established), which net/smtp.NewClient surfaces directly. +func TestEmailGreetingRejected(t *testing.T) { + l, err := net.Listen("tcp", "localhost:0") + require.NoError(t, err) + t.Cleanup(func() { _ = l.Close() }) + + done := make(chan any, 1) + go func() { + conn, err := l.Accept() + if err != nil { + close(done) + return + } + // A 421 greeting means the service is temporarily unavailable; net/smtp.NewClient + // returns this as a *textproto.Error before any session/session commands happen. + _, _ = conn.Write([]byte("421 Service not available, closing transmission channel\r\n")) + _ = conn.Close() + close(done) + }() + + require.IsType(t, &net.TCPAddr{}, l.Addr()) + addr := l.Addr().(*net.TCPAddr) + cfg := &config.EmailConfig{ + Smarthost: config.HostPort{Host: addr.IP.String(), Port: strconv.Itoa(addr.Port)}, + Hello: "localhost", + Headers: make(map[string]string), + From: "alertmanager@system", + To: "sre@company", + } + tmpl, firingAlert, err := prepare(cfg) + require.NoError(t, err) + + e := New(cfg, tmpl, promslog.NewNopLogger()) + + retry, err := e.Notify(context.Background(), firingAlert) + require.ErrorContains(t, err, "421") + require.True(t, retry) + + // A 421 (4xx) greeting is a temporary failure, which should surface as ServerErrorReason. + var reasonErr *notify.ErrorWithReason + require.ErrorAs(t, err, &reasonErr, "expected error to carry a notify.ErrorWithReason") + require.Equal(t, notify.ServerErrorReason, reasonErr.Reason) + + require.Eventuallyf(t, func() bool { + <-done + return true + }, time.Second*10, time.Millisecond*100, "mock listener goroutine failed to close in time") +} + func mockSMTPServer(t *testing.T) (*smtp.Server, net.Listener, error) { t.Helper()