Treat error response payloads from Slack as errors

As described in the "More error types" section below, Slack API can return
errors with a 200 response code:
https://slack.dev/node-slack-sdk/web-api#handle-errors

This change adds parsing of API response to extract error messages.

Signed-off-by: Anton Tolchanov <anton@tailscale.com>

notify/slack/slack.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
 	"os"
 	"strings"
@@ -88,6 +89,12 @@ type attachment struct {
 	MrkdwnIn   []string             `json:"mrkdwn_in,omitempty"`
 }
 
+// response is for parsing out errors from the JSON response.
+type response struct {
+	OK    bool   `json:"ok"`
+	Error string `json:"error"`
+}
+
 // Notify implements the Notifier interface.
 func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) {
 	var err error
@@ -206,12 +213,36 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error)
 	if err != nil {
 		return true, notify.RedactURL(err)
 	}
-	defer notify.Drain(resp)
+	defer resp.Body.Close()
 
 	// Only 5xx response codes are recoverable and 2xx codes are successful.
 	// https://api.slack.com/incoming-webhooks#handling_errors
 	// https://api.slack.com/changelog/2016-05-17-changes-to-errors-for-incoming-webhooks
 	retry, err := n.retrier.Check(resp.StatusCode, resp.Body)
 	err = errors.Wrap(err, fmt.Sprintf("channel %q", req.Channel))
+
+	if err == nil {
+		retry, err = respError(resp)
+	} else {
+		io.Copy(io.Discard, resp.Body)
+	}
+
 	return retry, err
 }
+
+// respError parses out the error message from Slack API response.
+func respError(resp *http.Response) (bool, error) {
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return true, errors.Wrap(err, "could not read response body")
+	}
+	var data response
+	err = json.Unmarshal(body, &data)
+	if err != nil {
+		return true, errors.Wrapf(err, "could not unmarshal JSON response %q", string(body))
+	}
+	if !data.OK {
+		return false, fmt.Errorf("error response from Slack: %s", data.Error)
+	}
+	return false, nil
+}

notify/slack/slack_test.go

@@ -14,16 +14,22 @@
 package slack
 
 import (
+	"context"
 	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
 	"os"
 	"testing"
 
 	"github.com/go-kit/log"
 	commoncfg "github.com/prometheus/common/config"
+	"github.com/prometheus/common/model"
 	"github.com/stretchr/testify/require"
 
 	"github.com/prometheus/alertmanager/config"
 	"github.com/prometheus/alertmanager/notify/test"
+	"github.com/prometheus/alertmanager/types"
 )
 
 func TestSlackRetry(t *testing.T) {
@@ -102,3 +108,44 @@ func TestTrimmingSlackURLFromFile(t *testing.T) {
 
 	test.AssertNotifyLeaksNoSecret(ctx, t, notifier, u.String())
 }
+
+func TestSlackErrorResponse(t *testing.T) {
+	var response *string
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Write([]byte(*response))
+	}))
+	u, err := url.Parse(srv.URL)
+	require.NoError(t, err)
+
+	notifier, err := New(
+		&config.SlackConfig{
+			APIURL:     &config.SecretURL{URL: u},
+			HTTPConfig: &commoncfg.HTTPClientConfig{},
+		},
+		test.CreateTmpl(t),
+		log.NewNopLogger(),
+	)
+	require.NoError(t, err)
+
+	for _, tt := range []struct {
+		name      string
+		response  string
+		wantRetry bool
+		wantErr   string
+	}{
+		{"no error", `{"ok":true,"error":"none"}`, false, ""},
+		// Slack web api returns errors with a 200 response code.
+		// https://slack.dev/node-slack-sdk/web-api#handle-errors
+		{"error response", `{"ok":false,"error":"channel_not_found"}`, false, "channel_not_found"},
+		{"invalid json", "plaintext_response", true, "plaintext_response"},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			response = &tt.response
+			retry, err := notifier.Notify(context.Background(), &types.Alert{Alert: model.Alert{Labels: model.LabelSet{"foo": "bar"}}})
+			require.Equal(t, tt.wantRetry, retry)
+			if tt.wantErr != "" {
+				require.ErrorContains(t, err, tt.wantErr)
+			}
+		})
+	}
+}