diff --git a/pkg/services/alerting/notifiers/slack.go b/pkg/services/alerting/notifiers/slack.go index 057227e79f7..72a7b7a2c4e 100644 --- a/pkg/services/alerting/notifiers/slack.go +++ b/pkg/services/alerting/notifiers/slack.go @@ -382,18 +382,22 @@ func (sn *SlackNotifier) sendRequest(ctx context.Context, data []byte) error { return fmt.Errorf("failed to read response body: %w", err) } - if resp.StatusCode >= http.StatusOK && resp.StatusCode < http.StatusMultipleChoices { + if resp.StatusCode >= 200 && resp.StatusCode < 300 { // Slack responds to some requests with a JSON document, that might contain an error rslt := struct { Ok bool `json:"ok"` Err string `json:"error"` }{} - if err := json.Unmarshal(body, &rslt); err == nil { - if !rslt.Ok && rslt.Err != "" { - sn.log.Warn("Sending Slack API request failed", "url", sn.url.String(), "statusCode", resp.Status, - "err", rslt.Err) - return fmt.Errorf("failed to make Slack API request: %s", rslt.Err) - } + if err := json.Unmarshal(body, &rslt); err != nil { + sn.log.Warn("Failed to unmarshal Slack API response", "url", sn.url.String(), "statusCode", resp.Status, + "err", err) + return fmt.Errorf("failed to unmarshal Slack API response with status code %d: %s", resp.StatusCode, err) + } + + if !rslt.Ok && rslt.Err != "" { + sn.log.Warn("Sending Slack API request failed", "url", sn.url.String(), "statusCode", resp.Status, + "err", rslt.Err) + return fmt.Errorf("failed to make Slack API request: %s", rslt.Err) } sn.log.Debug("Sending Slack API request succeeded", "url", sn.url.String(), "statusCode", resp.Status) diff --git a/pkg/services/alerting/notifiers/slack_test.go b/pkg/services/alerting/notifiers/slack_test.go index 64be9af4e32..d0a15199bbd 100644 --- a/pkg/services/alerting/notifiers/slack_test.go +++ b/pkg/services/alerting/notifiers/slack_test.go @@ -273,20 +273,20 @@ func TestSendSlackRequest(t *testing.T) { expectError: false, }, { - name: "Success case, no response body", + name: "No response body", statusCode: http.StatusOK, - expectError: false, + expectError: true, }, { name: "Success case, unexpected response body", statusCode: http.StatusOK, - slackResponse: "{}", + slackResponse: `{"test": true}`, expectError: false, }, { name: "Success case, ok: true", statusCode: http.StatusOK, - slackResponse: "{\"ok\": true}", + slackResponse: `{"ok": true}`, expectError: false, }, { diff --git a/pkg/services/ngalert/notifier/channels/slack.go b/pkg/services/ngalert/notifier/channels/slack.go index 85150eee979..04e9fb60247 100644 --- a/pkg/services/ngalert/notifier/channels/slack.go +++ b/pkg/services/ngalert/notifier/channels/slack.go @@ -218,7 +218,7 @@ var sendSlackRequest = func(request *http.Request, logger log.Logger) error { return fmt.Errorf("failed to read response body: %w", err) } - if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices { + if resp.StatusCode < 200 || resp.StatusCode >= 300 { logger.Warn("Slack API request failed", "url", request.URL.String(), "statusCode", resp.Status, "body", string(body)) return fmt.Errorf("request to Slack API failed with status code %d", resp.StatusCode) } @@ -228,12 +228,16 @@ var sendSlackRequest = func(request *http.Request, logger log.Logger) error { Ok bool `json:"ok"` Err string `json:"error"` }{} - if err := json.Unmarshal(body, &rslt); err == nil { - if !rslt.Ok && rslt.Err != "" { - logger.Warn("Sending Slack API request failed", "url", request.URL.String(), "statusCode", resp.Status, - "err", rslt.Err) - return fmt.Errorf("failed to make Slack API request: %s", rslt.Err) - } + if err := json.Unmarshal(body, &rslt); err != nil { + logger.Warn("Failed to unmarshal Slack API response", "url", request.URL.String(), "statusCode", resp.Status, + "body", string(body)) + return fmt.Errorf("failed to unmarshal Slack API response: %s", err) + } + + if !rslt.Ok && rslt.Err != "" { + logger.Warn("Sending Slack API request failed", "url", request.URL.String(), "statusCode", resp.Status, + "err", rslt.Err) + return fmt.Errorf("failed to make Slack API request: %s", rslt.Err) } logger.Debug("Sending Slack API request succeeded", "url", request.URL.String(), "statusCode", resp.Status) diff --git a/pkg/services/ngalert/notifier/channels/slack_test.go b/pkg/services/ngalert/notifier/channels/slack_test.go index cf2714dbbd5..2f137e40e8b 100644 --- a/pkg/services/ngalert/notifier/channels/slack_test.go +++ b/pkg/services/ngalert/notifier/channels/slack_test.go @@ -270,20 +270,20 @@ func TestSendSlackRequest(t *testing.T) { expectError: false, }, { - name: "Success case, no response body", + name: "No response body", statusCode: http.StatusOK, - expectError: false, + expectError: true, }, { name: "Success case, unexpected response body", statusCode: http.StatusOK, - slackResponse: "{}", + slackResponse: `{"test": true}`, expectError: false, }, { name: "Success case, ok: true", statusCode: http.StatusOK, - slackResponse: "{\"ok\": true}", + slackResponse: `{"ok": true}`, expectError: false, }, {