notify: a reply other than 2xx is a failed delivery (closes #287)
check / check (push) Canceled after 0s
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:
@@ -4,9 +4,12 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"maps"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"slices"
|
||||
"strings"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
@@ -536,3 +539,58 @@ 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)
|
||||
}
|
||||
|
||||
// For each endpoint, the messages of its lines whose error names
|
||||
// the 302, in the order they were logged.
|
||||
got := map[string][]string{}
|
||||
|
||||
for _, record := range parseLog(t, logs) {
|
||||
if strings.Contains(record.Error, "status 302") {
|
||||
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 302 = %v, want %v",
|
||||
got, want)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user