From de3663d3d9e6d13ddd1a7ea72bad8820b7188582 Mon Sep 17 00:00:00 2001 From: Max Nitze Date: Tue, 8 Sep 2026 16:45:26 +0200 Subject: [PATCH 1/3] notify/webex: drain and close the response body Signed-off-by: Max Nitze --- notify/webex/webex.go | 1 + 1 file changed, 1 insertion(+) diff --git a/notify/webex/webex.go b/notify/webex/webex.go index 949bb614f8..fe1bc477d0 100644 --- a/notify/webex/webex.go +++ b/notify/webex/webex.go @@ -105,6 +105,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) if err != nil { return true, notify.RedactURL(err) } + defer notify.Drain(resp) shouldRetry, err := n.retrier.Check(resp.StatusCode, resp.Body) if err != nil { From db275e79be3be4085d9b4236eaabc009717f060b Mon Sep 17 00:00:00 2001 From: Max Nitze Date: Tue, 8 Sep 2026 16:45:45 +0200 Subject: [PATCH 2/3] notify/webex: retry on 429 Signed-off-by: Max Nitze --- notify/webex/webex.go | 2 +- notify/webex/webex_test.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/notify/webex/webex.go b/notify/webex/webex.go index fe1bc477d0..84dfece8eb 100644 --- a/notify/webex/webex.go +++ b/notify/webex/webex.go @@ -54,7 +54,7 @@ func New(c *config.WebexConfig, t *template.Template, l *slog.Logger, httpOpts . tmpl: t, logger: l, client: client, - retrier: ¬ify.Retrier{}, + retrier: ¬ify.Retrier{RetryCodes: []int{http.StatusTooManyRequests}}, } return n, nil diff --git a/notify/webex/webex_test.go b/notify/webex/webex_test.go index eb12cfc2b6..029789a650 100644 --- a/notify/webex/webex_test.go +++ b/notify/webex/webex_test.go @@ -49,7 +49,8 @@ func TestWebexRetry(t *testing.T) { ) require.NoError(t, err) - for statusCode, expected := range test.RetryTests(test.DefaultRetryCodes()) { + retryCodes := append(test.DefaultRetryCodes(), http.StatusTooManyRequests) + for statusCode, expected := range test.RetryTests(retryCodes) { actual, _ := notifier.retrier.Check(statusCode, nil) require.Equal(t, expected, actual, "error on status %d", statusCode) } From 75f4006406e48d072ae783cd185971ad59f2bc18 Mon Sep 17 00:00:00 2001 From: Max Nitze Date: Tue, 8 Sep 2026 16:51:20 +0200 Subject: [PATCH 3/3] notify/webex: honor Retry-After header on 429 Signed-off-by: Max Nitze --- notify/webex/webex.go | 29 +++++++++++++- notify/webex/webex_test.go | 82 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/notify/webex/webex.go b/notify/webex/webex.go index 84dfece8eb..4e2e012b0e 100644 --- a/notify/webex/webex.go +++ b/notify/webex/webex.go @@ -19,6 +19,8 @@ import ( "encoding/json" "log/slog" "net/http" + "strconv" + "time" commoncfg "github.com/prometheus/common/config" @@ -105,12 +107,37 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) if err != nil { return true, notify.RedactURL(err) } - defer notify.Drain(resp) shouldRetry, err := n.retrier.Check(resp.StatusCode, resp.Body) + // Not deferred: Check has already consumed the body, and the connection must be + // released before the Retry-After wait below rather than held for its duration. + notify.Drain(resp) + if err != nil { + if resp.StatusCode == http.StatusTooManyRequests { + if d := parseRetryAfter(resp.Header.Get("Retry-After")); d > 0 { + logger.Warn("Rate limited by Webex, waiting before retry", "retry_after_secs", d.Seconds()) + select { + case <-time.After(d): + case <-ctx.Done(): + } + } + } return shouldRetry, notify.NewErrorWithReason(notify.GetFailureReasonFromStatusCode(resp.StatusCode), err) } return false, nil } + +// parseRetryAfter parses Retry-After as seconds; 0 if empty, invalid, or non-positive. +// TODO: switch to notify.ParseRetryAfter once upstream #5389 merges a shared helper. +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 +} diff --git a/notify/webex/webex_test.go b/notify/webex/webex_test.go index 029789a650..ef5e2d2315 100644 --- a/notify/webex/webex_test.go +++ b/notify/webex/webex_test.go @@ -170,6 +170,88 @@ func TestWebexTemplating(t *testing.T) { } } +func TestWebexRetryAfterSleep(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Retry-After", "1") + w.WriteHeader(http.StatusTooManyRequests) + })) + defer srv.Close() + u, err := url.Parse(srv.URL) + require.NoError(t, err) + + notifier, err := New( + &config.WebexConfig{ + HTTPConfig: &commoncfg.HTTPClientConfig{}, + APIURL: &amcommoncfg.URL{URL: u}, + }, + test.CreateTmpl(t), + promslog.NewNopLogger(), + ) + require.NoError(t, err) + + ctx := notify.WithGroupKey(context.Background(), "1") + alert := &types.Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"lbl1": "val1"}, + StartsAt: time.Now(), + EndsAt: time.Now().Add(time.Hour), + }, + } + + start := time.Now() + retry, err := notifier.Notify(ctx, alert) + elapsed := time.Since(start) + + require.True(t, retry) + require.Error(t, err) + require.GreaterOrEqual(t, elapsed, 1*time.Second, "should have waited at least 1 second for Retry-After") +} + +func TestWebexRetryAfterContextCancelled(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Retry-After", "2") + w.WriteHeader(http.StatusTooManyRequests) + })) + defer srv.Close() + u, err := url.Parse(srv.URL) + require.NoError(t, err) + + notifier, err := New( + &config.WebexConfig{ + HTTPConfig: &commoncfg.HTTPClientConfig{}, + APIURL: &amcommoncfg.URL{URL: u}, + }, + test.CreateTmpl(t), + promslog.NewNopLogger(), + ) + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + ctx = notify.WithGroupKey(ctx, "1") + + // Cancel context after a short delay to interrupt the Retry-After sleep. + go func() { + time.Sleep(100 * time.Millisecond) + cancel() + }() + + alert := &types.Alert{ + Alert: model.Alert{ + Labels: model.LabelSet{"lbl1": "val1"}, + StartsAt: time.Now(), + EndsAt: time.Now().Add(time.Hour), + }, + } + + start := time.Now() + retry, err := notifier.Notify(ctx, alert) + elapsed := time.Since(start) + + require.True(t, retry) + require.Error(t, err) + require.Less(t, elapsed, 2*time.Second, "should not have waited the full Retry-After duration") +} + func TestWebexFailureReason(t *testing.T) { for _, tc := range []struct { name string