diff --git a/notify/opsgenie/opsgenie.go b/notify/opsgenie/opsgenie.go index a8f3ba37bd..3e23a435fe 100644 --- a/notify/opsgenie/opsgenie.go +++ b/notify/opsgenie/opsgenie.go @@ -18,11 +18,14 @@ import ( "context" "encoding/json" "fmt" + "io" + "io/ioutil" "net/http" "strings" "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" + "github.com/pkg/errors" commoncfg "github.com/prometheus/common/config" "github.com/prometheus/common/model" @@ -85,7 +88,7 @@ func (n *Notifier) Notify(ctx context.Context, as ...*types.Alert) (bool, error) } defer notify.Drain(resp) - return n.retry(resp.StatusCode) + return n.retry(resp.StatusCode, resp.Body) } // Like Split but filter out empty strings. @@ -172,7 +175,7 @@ func (n *Notifier) createRequest(ctx context.Context, as ...*types.Alert) (*http apiKey := tmpl(string(n.conf.APIKey)) if err != nil { - return nil, false, fmt.Errorf("templating error: %s", err) + return nil, false, errors.Wrap(err, "templating error") } var buf bytes.Buffer @@ -189,14 +192,19 @@ func (n *Notifier) createRequest(ctx context.Context, as ...*types.Alert) (*http return req.WithContext(ctx), true, nil } -func (n *Notifier) retry(statusCode int) (bool, error) { - // https://docs.opsgenie.com/docs/response#section-response-codes - // Response codes 429 (rate limiting) and 5xx are potentially recoverable - if statusCode/100 == 5 || statusCode == 429 { - return true, fmt.Errorf("unexpected status code %v", statusCode) - } else if statusCode/100 != 2 { - return false, fmt.Errorf("unexpected status code %v", statusCode) +func (n *Notifier) retry(statusCode int, body io.Reader) (bool, error) { + if statusCode/100 == 2 { + return false, nil } - return false, nil + err := errors.Errorf("unexpected status code %v", statusCode) + if body != nil { + if bs, errRead := ioutil.ReadAll(body); errRead == nil { + err = errors.Errorf("%s: %s", err, string(bs)) + } + } + + // https://docs.opsgenie.com/docs/response#section-response-codes + // Response codes 429 (rate limiting) and 5xx are potentially recoverable + return statusCode/100 == 5 || statusCode == 429, err } diff --git a/notify/opsgenie/opsgenie_test.go b/notify/opsgenie/opsgenie_test.go index 46be6b1d7e..fc16c06420 100644 --- a/notify/opsgenie/opsgenie_test.go +++ b/notify/opsgenie/opsgenie_test.go @@ -14,8 +14,10 @@ package opsgenie import ( + "bytes" "context" "fmt" + "io" "io/ioutil" "net/http" "net/url" @@ -38,11 +40,41 @@ func TestOpsGenieRetry(t *testing.T) { retryCodes := append(test.DefaultRetryCodes(), http.StatusTooManyRequests) for statusCode, expected := range test.RetryTests(retryCodes) { - actual, _ := notifier.retry(statusCode) + actual, _ := notifier.retry(statusCode, nil) require.Equal(t, expected, actual, fmt.Sprintf("error on status %d", statusCode)) } } +func TestOpsGenieErr(t *testing.T) { + notifier := new(Notifier) + for _, tc := range []struct { + status int + body io.Reader + expected string + }{ + { + status: http.StatusUnprocessableEntity, + body: nil, + expected: "unexpected status code 422", + }, + { + status: http.StatusUnprocessableEntity, + body: bytes.NewBuffer([]byte(`{"message":"Request body is not processable. Please check the errors.","errors":{"priority":"should be one of [ P1, P2, P3, P4, P5 ]"},"took":0.002,"requestId":"865a4f83-99d9-48c8-9550-42a375a3a387"}`)), + expected: `unexpected status code 422: {"message":"Request body is not processable. Please check the errors.","errors":{"priority":"should be one of [ P1, P2, P3, P4, P5 ]"},"took":0.002,"requestId":"865a4f83-99d9-48c8-9550-42a375a3a387"}`, + }, + { + status: http.StatusInternalServerError, + body: bytes.NewBuffer([]byte("internal error")), + expected: "unexpected status code 500: internal error", + }, + } { + t.Run("", func(t *testing.T) { + _, err := notifier.retry(tc.status, tc.body) + require.Equal(t, err.Error(), tc.expected) + }) + } +} + func TestOpsGenieRedactedURL(t *testing.T) { ctx, u, fn := test.GetContextWithCancelingURL() defer fn()