notify: a reply other than 2xx is a failed delivery (closes #287)
check / check (push) Canceled after 0s

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
This commit is contained in:
2026-10-15 12:55:17 +00:00
parent 5e2c456e85
commit a1caf4914c
5 changed files with 81 additions and 36 deletions
+37 -27
View File
@@ -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)
// 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)
}
assertEachEndpointRetriedThenFailed(t, logs, "connection refused")
}
// ── SlackPayload JSON marshaling ──────────────────────────