notify: a reply other than 2xx is a failed delivery (closes #287)

A notification endpoint that answered with a redirect was counted as
delivered: the request is not sent through a client, so the redirect
is never followed, and only a status of 400 or more counted as a
failure. Now any status outside 200-299 fails the delivery for ntfy,
Slack and Mattermost, so it is retried and logged like the others.

Model: opus-5-5

README.md

@@ -286,7 +286,7 @@ includes:
 
 Each endpoint is sent each notification on its own, in the background. A
 delivery that fails (a network error, no reply within 10 seconds, or an HTTP
-status of 400 or more) is retried up to 5 times: the first retry after about 1
+status other than 2xx) is retried up to 5 times: the first retry after about 1
 second, each wait after that twice as long up to 60 seconds, every wait varied
 at random by up to 25%. A delivery still failing after that is logged and
 dropped.

TODO.md

@@ -19,6 +19,8 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
 
 # Completed Steps
 
+- 2026-10-15: a notification endpoint that answers with a redirect, or any
+  status other than 2xx, is a failed delivery, retried and logged (closes #287).
 - 2026-10-15: a nameserver is asked at each of its IPv4 addresses in turn, so it
   has failed only when none of them gives a usable reply (closes #285).
 - 2026-10-15: the log names a notification endpoint (`slack`, `mattermost`,

internal/notify/delivery_test.go

@@ -1185,6 +1185,42 @@ func sendToEachEndpoint(t *testing.T, baseURL, title string) string {
 	return logs.String()
 }
 
+// assertEachEndpointRetriedThenFailed fails the test unless, for each
+// of ntfy, Slack and Mattermost, the lines of logs whose error
+// contains reason are the retry line and then the final failure line.
+func assertEachEndpointRetriedThenFailed(
+	t *testing.T, logs, reason string,
+) {
+	t.Helper()
+
+	// For each endpoint, the messages of its lines whose error
+	// contains reason, in the order they were logged.
+	got := map[string][]string{}
+
+	for _, record := range parseLog(t, logs) {
+		if strings.Contains(record.Error, reason) {
+			got[record.Endpoint] = append(
+				got[record.Endpoint], record.Msg,
+			)
+		}
+	}
+
+	lines := []string{
+		"notification delivery failed, retrying",
+		"failed to send notification after retries",
+	}
+	want := map[string][]string{
+		endpointNtfy:       lines,
+		endpointSlack:      lines,
+		endpointMattermost: lines,
+	}
+
+	if !maps.EqualFunc(got, want, slices.Equal) {
+		t.Errorf("lines whose error contains %q = %v, want %v",
+			reason, got, want)
+	}
+}
+
 // TestSendNotificationDebugLogOmitsWebhookURL checks that, with debug
 // logging on, a delivery to each endpoint logs the endpoint's name and
 // the title, and no part of the URL's path or query.
@@ -1240,33 +1276,7 @@ func TestSendNotificationErrorLogOmitsWebhookURL(t *testing.T) {
 	logs := sendToEachEndpoint(t, srv.URL, "t")
 
 	assertNoWebhookSecret(t, logs)
-
+	assertEachEndpointRetriedThenFailed(t, logs, "connection refused")
-	// For each endpoint, the messages of its lines whose error names
-	// the refused connection, in the order they were logged.
-	got := map[string][]string{}
-
-	for _, record := range parseLog(t, logs) {
-		if strings.Contains(record.Error, "connection refused") {
-			got[record.Endpoint] = append(
-				got[record.Endpoint], record.Msg,
-			)
-		}
-	}
-
-	lines := []string{
-		"notification delivery failed, retrying",
-		"failed to send notification after retries",
-	}
-	want := map[string][]string{
-		endpointNtfy:       lines,
-		endpointSlack:      lines,
-		endpointMattermost: lines,
-	}
-
-	if !maps.EqualFunc(got, want, slices.Equal) {
-		t.Errorf("lines whose error names the refused connection = %v, "+
-			"want %v", got, want)
-	}
 }
 
 // ── SlackPayload JSON marshaling ──────────────────────────

internal/notify/notify.go

@@ -25,9 +25,6 @@ import (
 // HTTP client timeout.
 const httpClientTimeout = 10 * time.Second
 
-// HTTP status code thresholds.
-const httpStatusClientError = 400
-
 // Sentinel errors for notification failures.
 var (
 	// ErrNtfyFailed indicates the ntfy request failed.
@@ -344,7 +341,10 @@ func (svc *Service) sendNtfy(
 
 	defer func() { _ = resp.Body.Close() }()
 
-	if resp.StatusCode >= httpStatusClientError {
+	// Only a 2xx reply means ntfy took the notification. A
+	// redirect is not followed, so it is a failure too.
+	if resp.StatusCode < http.StatusOK ||
+		resp.StatusCode >= http.StatusMultipleChoices {
 		return fmt.Errorf(
 			"%w: status %d",
 			ErrNtfyFailed, resp.StatusCode,
@@ -385,9 +385,9 @@ type SlackAttachment struct {
 // sendSlack posts to a Slack or Mattermost incoming webhook, which
 // take the same payload. endpoint, "slack" or "mattermost", names
 // the webhook in the log in place of its URL, which lets anyone who
-// has it post to the channel. An HTTP error status is returned
+// has it post to the channel. A reply status other than 2xx is
-// wrapped in failed, ErrSlackFailed or ErrMattermostFailed, so the
+// returned wrapped in failed, ErrSlackFailed or ErrMattermostFailed,
-// error names the endpoint.
+// so the error names the endpoint.
 func (svc *Service) sendSlack(
 	ctx context.Context,
 	webhookURL *url.URL,
@@ -437,7 +437,10 @@ func (svc *Service) sendSlack(
 
 	defer func() { _ = resp.Body.Close() }()
 
-	if resp.StatusCode >= httpStatusClientError {
+	// Only a 2xx reply means the webhook took the notification. A
+	// redirect is not followed, so it is a failure too.
+	if resp.StatusCode < http.StatusOK ||
+		resp.StatusCode >= http.StatusMultipleChoices {
 		return fmt.Errorf(
 			"%w: status %d",
 			failed, resp.StatusCode,

internal/notify/retry_test.go

@@ -536,3 +536,33 @@ func TestSendNotificationPermanentFailureLogsError(
 		return attempts >= 3
 	})
 }
+
+// TestSendNotificationRedirectFails checks that a 302 reply is a
+// failed delivery at each endpoint: the server is sent the first
+// attempt and the retry, and each endpoint logs a retry line and
+// then the final failure line, both naming the 302.
+func TestSendNotificationRedirectFails(t *testing.T) {
+	t.Parallel()
+
+	var requests atomic.Int64
+
+	srv := httptest.NewServer(
+		http.HandlerFunc(
+			func(w http.ResponseWriter, r *http.Request) {
+				requests.Add(1)
+				http.Redirect(w, r, "/moved", http.StatusFound)
+			}),
+	)
+	defer srv.Close()
+
+	logs := sendToEachEndpoint(t, srv.URL, "t")
+
+	// ntfy, Slack and Mattermost each send once and retry once.
+	const wantRequests = 3 * 2
+
+	if sent := requests.Load(); sent != wantRequests {
+		t.Errorf("server got %d requests, want %d", sent, wantRequests)
+	}
+
+	assertEachEndpointRetriedThenFailed(t, logs, "status 302")
+}