diff --git a/notify/pushover/pushover.go b/notify/pushover/pushover.go index eae0af0075..d42d558d5b 100644 --- a/notify/pushover/pushover.go +++ b/notify/pushover/pushover.go @@ -130,5 +130,5 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) } defer notify.Drain(resp) - return n.retrier.Check(resp.StatusCode, nil) + return n.retrier.Check(resp.StatusCode, resp.Body) } diff --git a/notify/victorops/victorops.go b/notify/victorops/victorops.go index 3878c1103b..7bab2e5cec 100644 --- a/notify/victorops/victorops.go +++ b/notify/victorops/victorops.go @@ -86,7 +86,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) } defer notify.Drain(resp) - return n.retrier.Check(resp.StatusCode, nil) + return n.retrier.Check(resp.StatusCode, resp.Body) } // Create the JSON payload to be sent to the VictorOps API. diff --git a/notify/webhook/webhook.go b/notify/webhook/webhook.go index 00c525024b..463a3416f5 100644 --- a/notify/webhook/webhook.go +++ b/notify/webhook/webhook.go @@ -17,6 +17,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "io" "net/http" @@ -53,8 +54,8 @@ func New(conf *config.WebhookConfig, t *template.Template, l log.Logger, httpOpt // Webhooks are assumed to respond with 2xx response codes on a successful // request and 5xx response codes are assumed to be recoverable. retrier: ¬ify.Retrier{ - CustomDetailsFunc: func(int, io.Reader) string { - return conf.URL.String() + CustomDetailsFunc: func(_ int, body io.Reader) string { + return errDetails(body, conf.URL.String()) }, }, }, nil @@ -104,7 +105,18 @@ func (n *Notifier) Notify(ctx context.Context, alerts ...*types.Alert) (bool, er if err != nil { return true, err } - notify.Drain(resp) + defer notify.Drain(resp) - return n.retrier.Check(resp.StatusCode, nil) + return n.retrier.Check(resp.StatusCode, resp.Body) +} + +func errDetails(body io.Reader, url string) string { + if body == nil { + return url + } + bs, err := io.ReadAll(body) + if err != nil { + return url + } + return fmt.Sprintf("%s: %s", url, string(bs)) } diff --git a/notify/webhook/webhook_test.go b/notify/webhook/webhook_test.go index facfd6ff05..116ee1c2b3 100644 --- a/notify/webhook/webhook_test.go +++ b/notify/webhook/webhook_test.go @@ -14,7 +14,10 @@ package webhook import ( + "bytes" "fmt" + "io" + "net/http" "net/url" "testing" @@ -43,10 +46,41 @@ func TestWebhookRetry(t *testing.T) { if err != nil { require.NoError(t, err) } - for statusCode, expected := range test.RetryTests(test.DefaultRetryCodes()) { - actual, _ := notifier.retrier.Check(statusCode, nil) - require.Equal(t, expected, actual, fmt.Sprintf("error on status %d", statusCode)) - } + + t.Run("test retry status code", func(t *testing.T) { + for statusCode, expected := range test.RetryTests(test.DefaultRetryCodes()) { + actual, _ := notifier.retrier.Check(statusCode, nil) + require.Equal(t, expected, actual, fmt.Sprintf("error on status %d", statusCode)) + } + }) + + t.Run("test retry error details", func(t *testing.T) { + for _, tc := range []struct { + status int + body io.Reader + + exp string + }{ + { + status: http.StatusBadRequest, + body: bytes.NewBuffer([]byte( + `{"status":"invalid event"}`, + )), + + exp: fmt.Sprintf(`unexpected status code %d: %s: {"status":"invalid event"}`, http.StatusBadRequest, u.String()), + }, + { + status: http.StatusBadRequest, + + exp: fmt.Sprintf(`unexpected status code %d: %s`, http.StatusBadRequest, u.String()), + }, + } { + t.Run("", func(t *testing.T) { + _, err = notifier.retrier.Check(tc.status, tc.body) + require.Equal(t, tc.exp, err.Error()) + }) + } + }) } func TestWebhookTruncateAlerts(t *testing.T) {