diff --git a/notify/slack/slack.go b/notify/slack/slack.go index 64964db4ea..adfe53acdd 100644 --- a/notify/slack/slack.go +++ b/notify/slack/slack.go @@ -22,7 +22,6 @@ import ( "log/slog" "net/http" "os" - "strconv" "strings" "time" @@ -198,6 +197,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) } resp, err := n.postJSONFunc(ctx, n.client, u, &buf) + received := time.Now() if err != nil { if ctx.Err() != nil { err = fmt.Errorf("%w: %w", err, context.Cause(ctx)) @@ -211,7 +211,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) retry, err := n.retrier.Check(resp.StatusCode, resp.Body) if err != nil { if resp.StatusCode == http.StatusTooManyRequests { - if d := parseRetryAfter(resp.Header.Get("Retry-After")); d > 0 { + if d := notify.ParseRetryAfter(resp.Header, received); d > 0 { n.logger.Warn("Rate limited by Slack, waiting before retry", "retry_after_secs", d.Seconds()) select { case <-time.After(d): @@ -257,20 +257,6 @@ func (n *Notifier) slackResponseHandler(resp *http.Response, store *nflog.Store) return false, nil } -// parseRetryAfter parses the Retry-After header value as integer seconds -// and returns the corresponding duration. Returns 0 if the value is empty, -// not a valid integer, or non-positive. -func parseRetryAfter(val string) time.Duration { - if val == "" { - return 0 - } - seconds, err := strconv.Atoi(val) - if err != nil || seconds <= 0 { - return 0 - } - return time.Duration(seconds) * time.Second -} - // checkTextResponseError classifies plaintext responses from Slack. // A plaintext (non-JSON) response is successful if it's a string "ok". // This is typically a response for an Incoming Webhook diff --git a/notify/slack/slack_test.go b/notify/slack/slack_test.go index ba0e793907..076d1d7ca7 100644 --- a/notify/slack/slack_test.go +++ b/notify/slack/slack_test.go @@ -361,26 +361,6 @@ func TestSlackMessageField(t *testing.T) { } } -func TestParseRetryAfter(t *testing.T) { - tests := []struct { - name string - value string - expected time.Duration - }{ - {name: "valid integer", value: "30", expected: 30 * time.Second}, - {name: "empty string", value: "", expected: 0}, - {name: "non-integer", value: "abc", expected: 0}, - {name: "negative", value: "-5", expected: 0}, - {name: "zero", value: "0", expected: 0}, - {name: "float value", value: "1.5", expected: 0}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - require.Equal(t, tt.expected, parseRetryAfter(tt.value)) - }) - } -} - func TestNotifier_Notify_RetryAfterSleep(t *testing.T) { apiurl, _ := url.Parse("https://slack.com/post.Message") notifier, err := New( diff --git a/notify/util.go b/notify/util.go index fe4c9ea508..38d58ffc68 100644 --- a/notify/util.go +++ b/notify/util.go @@ -23,7 +23,9 @@ import ( "net/http" "net/url" "slices" + "strconv" "strings" + "time" commoncfg "github.com/prometheus/common/config" "github.com/prometheus/common/version" @@ -239,6 +241,30 @@ type Retrier struct { RetryCodes []int } +// ParseRetryAfter parses the Retry-After header value, which can be either +// a delay in seconds (integer) or an HTTP-date. Returns zero if absent or unparseable. +func ParseRetryAfter(h http.Header, received time.Time) time.Duration { + val := h.Get("Retry-After") + if val == "" { + return 0 + } + // Try integer seconds first. + if secs, err := strconv.Atoi(val); err == nil { + // The grammar is 1*DIGIT, so a negative is malformed rather than a delay. + return max(0, time.Duration(secs)*time.Second) + } + // Try HTTP-date format. + if t, err := http.ParseTime(val); err == nil { + // The date is on the server's clock, so time.Until would apply our skew. + now := received + if serverNow, err := http.ParseTime(h.Get("Date")); err == nil { + now = serverNow + } + return max(0, t.Sub(now)) + } + return 0 +} + // Check returns a boolean indicating whether the request should be retried // and an optional error if the request has failed. If body is not nil, it will // be included in the error message. diff --git a/notify/util_test.go b/notify/util_test.go index 2c2d4922e8..372894a2f3 100644 --- a/notify/util_test.go +++ b/notify/util_test.go @@ -24,6 +24,7 @@ import ( "reflect" "runtime" "testing" + "time" "github.com/prometheus/common/model" "github.com/prometheus/common/promslog" @@ -273,3 +274,57 @@ func TestGetFailureReasonFromStatusCode(t *testing.T) { }) } } + +func TestParseRetryAfter(t *testing.T) { + received := time.Date(2026, 9, 10, 12, 0, 0, 0, time.UTC) + // A client whose clock trails the server's must still get the delay right. + serverNow := received.Add(time.Minute) + + header := func(retryAfter, date string) http.Header { + h := http.Header{} + if retryAfter != "" { + h.Set("Retry-After", retryAfter) + } + if date != "" { + h.Set("Date", date) + } + return h + } + + for _, tc := range []struct { + name string + header http.Header + expected time.Duration + }{ + {name: "absent", header: header("", ""), expected: 0}, + {name: "valid integer", header: header("30", ""), expected: 30 * time.Second}, + {name: "non-integer", header: header("abc", ""), expected: 0}, + {name: "negative", header: header("-5", ""), expected: 0}, + {name: "zero", header: header("0", ""), expected: 0}, + {name: "float value", header: header("1.5", ""), expected: 0}, + { + name: "date against server clock", + header: header(serverNow.Add(30*time.Second).Format(http.TimeFormat), serverNow.Format(http.TimeFormat)), + expected: 30 * time.Second, + }, + { + name: "date without Date header", + header: header(received.Add(30*time.Second).Format(http.TimeFormat), ""), + expected: 30 * time.Second, + }, + { + name: "date with unparseable Date header", + header: header(received.Add(30*time.Second).Format(http.TimeFormat), "not a date"), + expected: 30 * time.Second, + }, + { + name: "date already elapsed", + header: header(serverNow.Add(-30*time.Second).Format(http.TimeFormat), serverNow.Format(http.TimeFormat)), + expected: 0, + }, + } { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expected, ParseRetryAfter(tc.header, received)) + }) + } +}