From e3c632c793debd6108a895073ff9f518d63c9528 Mon Sep 17 00:00:00 2001 From: sneak Date: Fri, 2 Oct 2026 06:14:40 +0000 Subject: [PATCH] notify: a failed Mattermost delivery's error names Mattermost (closes #227) Mattermost is sent by the Slack sender, which wrapped every HTTP error status in ErrSlackFailed, so a Mattermost endpoint answering 503 was logged as "slack notification failed". The sender now takes the error to wrap: the Slack endpoint passes ErrSlackFailed and the Mattermost endpoint passes ErrMattermostFailed, which was defined but unused. A new delivery test sets both endpoints to a stand-in server answering 503 and checks the error logged for each names its own endpoint. Model: opus-5-5 --- TODO.md | 2 + internal/notify/delivery_test.go | 72 ++++++++++++++++++++++++++++++-- internal/notify/export_test.go | 3 +- internal/notify/notify.go | 12 ++++-- 4 files changed, 82 insertions(+), 7 deletions(-) diff --git a/TODO.md b/TODO.md index 6de64a5..baedf40 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149 # Completed Steps +- 2026-10-02: a Mattermost webhook that answers an HTTP error is logged as + `mattermost notification failed`, not as a Slack failure (closes #227). - 2026-10-02: durations in the log are written as text such as `2m0s`, not as a bare count of nanoseconds (closes #228). - 2026-10-02: a watched name whose nameservers answer with a CNAME and no diff --git a/internal/notify/delivery_test.go b/internal/notify/delivery_test.go index fdccd73..064af7b 100644 --- a/internal/notify/delivery_test.go +++ b/internal/notify/delivery_test.go @@ -6,9 +6,11 @@ import ( "encoding/json" "errors" "io" + "maps" "net/http" "net/http/httptest" "net/url" + "strings" "sync" "testing" "time" @@ -413,7 +415,8 @@ func sendSlackInfo( svc *notify.Service, target *url.URL, ) error { return svc.SendSlack( - context.Background(), target, "t", "m", prioInfo, + context.Background(), target, notify.ErrSlackFailed, + "t", "m", prioInfo, ) } @@ -506,6 +509,7 @@ func TestSendSlackPayloadFields(t *testing.T) { err := svc.SendSlack( context.Background(), webhookURL, + notify.ErrSlackFailed, "Alert Title", "Alert body text", "warning", @@ -608,7 +612,8 @@ func TestSendSlackAllColors(t *testing.T) { err := svc.SendSlack( context.Background(), - webhookURL, "t", "m", tc.priority, + webhookURL, notify.ErrSlackFailed, + "t", "m", tc.priority, ) if err != nil { t.Fatalf("SendSlack error: %v", err) @@ -659,7 +664,8 @@ func TestSendSlackNetworkError(t *testing.T) { ) err := svc.SendSlack( - context.Background(), webhookURL, "t", "m", "info", + context.Background(), webhookURL, notify.ErrSlackFailed, + "t", "m", "info", ) if err == nil { t.Fatal("expected error for network failure") @@ -1028,6 +1034,66 @@ func TestSendNotificationMattermostError(t *testing.T) { ) } +// TestSendNotificationErrorNamesEndpoint verifies that, with both +// Slack and Mattermost set, a failed delivery's logged error names +// the endpoint that failed. Both are sent by the Slack sender. +func TestSendNotificationErrorNamesEndpoint(t *testing.T) { + t.Parallel() + + srv := httptest.NewServer( + http.HandlerFunc( + func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusServiceUnavailable) + }), + ) + defer srv.Close() + + target, _ := url.Parse(srv.URL) + + svc, logs := newLoggingService(http.DefaultTransport) + svc.SetSlackWebhookURL(target) + svc.SetMattermostWebhookURL(target) + svc.SetSleepFunc(instantSleep) + svc.SetRetryConfig(notify.RetryConfig{ + MaxRetries: 1, + BaseDelay: time.Millisecond, + MaxDelay: time.Millisecond, + }) + + svc.SendNotification( + context.Background(), "t", "m", prioError, + ) + + waitForCondition(t, func() bool { + return svc.OutstandingDeliveries() == 0 + }) + + got := map[string]string{} + + for line := range strings.Lines(logs.String()) { + var record struct { + Msg string `json:"msg"` + Endpoint string `json:"endpoint"` + Error string `json:"error"` + } + + _ = json.Unmarshal([]byte(line), &record) + + if record.Msg == "failed to send notification after retries" { + got[record.Endpoint] = record.Error + } + } + + want := map[string]string{ + "slack": "slack notification failed: status 503", + "mattermost": "mattermost notification failed: status 503", + } + + if !maps.Equal(got, want) { + t.Errorf("logged errors = %v, want %v", got, want) + } +} + // ── SlackPayload JSON marshaling ────────────────────────── func TestSlackPayloadJSON(t *testing.T) { diff --git a/internal/notify/export_test.go b/internal/notify/export_test.go index 723fee6..52c3b3d 100644 --- a/internal/notify/export_test.go +++ b/internal/notify/export_test.go @@ -85,10 +85,11 @@ func (svc *Service) SendNtfy( func (svc *Service) SendSlack( ctx context.Context, webhookURL *url.URL, + failed error, title, message, priority string, ) error { return svc.sendSlack( - ctx, webhookURL, title, message, priority, + ctx, webhookURL, failed, title, message, priority, ) } diff --git a/internal/notify/notify.go b/internal/notify/notify.go index 1185ed6..ff9026a 100644 --- a/internal/notify/notify.go +++ b/internal/notify/notify.go @@ -277,7 +277,8 @@ func (svc *Service) dispatchSlack( svc.dispatch(ctx, "slack", func(c context.Context) error { return svc.sendSlack( - c, svc.slackWebhookURL, title, message, priority, + c, svc.slackWebhookURL, ErrSlackFailed, + title, message, priority, ) }) } @@ -294,7 +295,7 @@ func (svc *Service) dispatchMattermost( ctx, "mattermost", func(c context.Context) error { return svc.sendSlack( - c, svc.mattermostWebhookURL, + c, svc.mattermostWebhookURL, ErrMattermostFailed, title, message, priority, ) }, @@ -370,9 +371,14 @@ type SlackAttachment struct { Text string `json:"text"` } +// sendSlack posts to a Slack or Mattermost incoming webhook, which +// take the same payload. An HTTP error status is returned wrapped in +// failed, ErrSlackFailed or ErrMattermostFailed, so the error names +// the endpoint. func (svc *Service) sendSlack( ctx context.Context, webhookURL *url.URL, + failed error, title, message, priority string, ) error { ctx, cancel := context.WithTimeout( @@ -420,7 +426,7 @@ func (svc *Service) sendSlack( if resp.StatusCode >= httpStatusClientError { return fmt.Errorf( "%w: status %d", - ErrSlackFailed, resp.StatusCode, + failed, resp.StatusCode, ) } -- 2.54.0