IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Fix slack contact point panic (code review changes) (#40770)

Browse Source 
* panic when unexpected Slack response fixed, tests added

* fix linting errors, add test

* Code review changes

* Apply PR comments

Co-authored-by: Armand Grillet <armand.grillet@outlook.com>
This commit is contained in:
Santiago 2021-10-22 12:23:22 -03:00 committed by GitHub
parent 0a97d0fff3
commit 1095f69740
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 30 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
pkg/services/alerting/notifiers/slack.go
View File

@ -382,18 +382,22 @@ func (sn *SlackNotifier) sendRequest(ctx context.Context, data []byte) error {
return fmt.Errorf("failed to read response body: %w", err) 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 // Slack responds to some requests with a JSON document, that might contain an error
rslt := struct { rslt := struct {
Ok bool `json:"ok"` Ok bool `json:"ok"`
Err string `json:"error"` Err string `json:"error"`
}{} }{}
if err := json.Unmarshal(body, &rslt); err == nil { if err := json.Unmarshal(body, &rslt); err != nil {
if !rslt.Ok && rslt.Err != "" { sn.log.Warn("Failed to unmarshal Slack API response", "url", sn.url.String(), "statusCode", resp.Status,
sn.log.Warn("Sending Slack API request failed", "url", sn.url.String(), "statusCode", resp.Status, "err", err)
"err", rslt.Err) return fmt.Errorf("failed to unmarshal Slack API response with status code %d: %s", resp.StatusCode, err)
return fmt.Errorf("failed to make Slack API request: %s", rslt.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) sn.log.Debug("Sending Slack API request succeeded", "url", sn.url.String(), "statusCode", resp.Status)

8
pkg/services/alerting/notifiers/slack_test.go
View File

@ -273,20 +273,20 @@ func TestSendSlackRequest(t *testing.T) {
expectError: false, expectError: false,
}, },
{ {
name: "Success case, no response body", name: "No response body",
statusCode: http.StatusOK, statusCode: http.StatusOK,
expectError: false, expectError: true,
}, },
{ {
name: "Success case, unexpected response body", name: "Success case, unexpected response body",
statusCode: http.StatusOK, statusCode: http.StatusOK,
slackResponse: "{}", slackResponse: `{"test": true}`,
expectError: false, expectError: false,
}, },
{ {
name: "Success case, ok: true", name: "Success case, ok: true",
statusCode: http.StatusOK, statusCode: http.StatusOK,
slackResponse: "{\"ok\": true}", slackResponse: `{"ok": true}`,
expectError: false, expectError: false,
}, },
{ {

18
pkg/services/ngalert/notifier/channels/slack.go
View File

@ -218,7 +218,7 @@ var sendSlackRequest = func(request *http.Request, logger log.Logger) error {
return fmt.Errorf("failed to read response body: %w", err) 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)) 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) 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"` Ok bool `json:"ok"`
Err string `json:"error"` Err string `json:"error"`
}{} }{}
if err := json.Unmarshal(body, &rslt); err == nil { if err := json.Unmarshal(body, &rslt); err != nil {
if !rslt.Ok && rslt.Err != "" { logger.Warn("Failed to unmarshal Slack API response", "url", request.URL.String(), "statusCode", resp.Status,
logger.Warn("Sending Slack API request failed", "url", request.URL.String(), "statusCode", resp.Status, "body", string(body))
"err", rslt.Err) return fmt.Errorf("failed to unmarshal Slack API response: %s", err)
return fmt.Errorf("failed to make Slack API request: %s", rslt.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) logger.Debug("Sending Slack API request succeeded", "url", request.URL.String(), "statusCode", resp.Status)

8
pkg/services/ngalert/notifier/channels/slack_test.go
View File

@ -270,20 +270,20 @@ func TestSendSlackRequest(t *testing.T) {
expectError: false, expectError: false,
}, },
{ {
name: "Success case, no response body", name: "No response body",
statusCode: http.StatusOK, statusCode: http.StatusOK,
expectError: false, expectError: true,
}, },
{ {
name: "Success case, unexpected response body", name: "Success case, unexpected response body",
statusCode: http.StatusOK, statusCode: http.StatusOK,
slackResponse: "{}", slackResponse: `{"test": true}`,
expectError: false, expectError: false,
}, },
{ {
name: "Success case, ok: true", name: "Success case, ok: true",
statusCode: http.StatusOK, statusCode: http.StatusOK,
slackResponse: "{\"ok\": true}", slackResponse: `{"ok": true}`,
expectError: false, expectError: false,
}, },
{ {