From e7da911aeb6bb0e6667557e7230c459f5c91abb1 Mon Sep 17 00:00:00 2001
From: Oktarian Tilney-Bassett <oktariantilneybassett@improbable.io>
Date: Fri, 14 Oct 2022 16:13:41 +0100
Subject: [PATCH] Custom error details for webhook notifier

Signed-off-by: Oktarian Tilney-Bassett <oktariantilneybassett@improbable.io>
---
 notify/webhook/webhook.go      | 16 +++++++++++--
 notify/webhook/webhook_test.go | 42 ++++++++++++++++++++++++++++++----
 2 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/notify/webhook/webhook.go b/notify/webhook/webhook.go
index a4754e954c..c86c604675 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: &notify.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
@@ -108,3 +109,14 @@ func (n *Notifier) Notify(ctx context.Context, alerts ...*types.Alert) (bool, er
 
 	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 at %s", string(bs), url)
+}
diff --git a/notify/webhook/webhook_test.go b/notify/webhook/webhook_test.go
index facfd6ff05..edef64af09 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: {"status":"invalid event"} at %s`, 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) {