From 7e2356af3ae8526235c9c4ebaf0420c9fd64b673 Mon Sep 17 00:00:00 2001 From: Alin Sinpalean <58422065+alin-at-dfinity@users.noreply.github.com> Date: Fri, 17 Feb 2023 10:22:23 +0100 Subject: [PATCH] Better Jira error handling (#140) * Better Jira error handling * Return HTTP 400 Bad Request for non-retriable errors. It is inaccurate, but will prevent alertmanager from retrying. * Turns out go-jira does actually produce useful error messages (and it consumes the response body in the process). Log that instead of the empty body. Signed-off-by: Alin Sinpalean * Also include HTTP response status 429 among retriable errors. Signed-off-by: Alin Sinpalean * Include both the go-jira error and the response body in errors. Sometimes go-jira consumes the body and includes it in its error, sometimes it doesn't. Signed-off-by: Alin Sinpalean --------- Signed-off-by: Alin Sinpalean Co-authored-by: Alin Sinpalean --- cmd/jiralert/main.go | 5 +++-- pkg/notify/notify.go | 9 +++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/cmd/jiralert/main.go b/cmd/jiralert/main.go index 2351203..a850570 100644 --- a/cmd/jiralert/main.go +++ b/cmd/jiralert/main.go @@ -17,12 +17,12 @@ import ( "encoding/json" "flag" "fmt" - "github.com/andygrunwald/go-jira" "net/http" "os" "runtime" "strconv" + "github.com/andygrunwald/go-jira" "github.com/go-kit/log" "github.com/go-kit/log/level" "github.com/prometheus-community/jiralert/pkg/alertmanager" @@ -127,7 +127,8 @@ func main() { // Instruct Alertmanager to retry. status = http.StatusServiceUnavailable } else { - status = http.StatusInternalServerError + // Inaccurate, just letting Alertmanager know that it should not retry. + status = http.StatusBadRequest } errorHandler(w, status, err, conf.Name, &data, logger) return diff --git a/pkg/notify/notify.go b/pkg/notify/notify.go index 00a2d9e..319bdeb 100644 --- a/pkg/notify/notify.go +++ b/pkg/notify/notify.go @@ -17,12 +17,12 @@ import ( "bytes" "crypto/sha512" "fmt" - "github.com/andygrunwald/go-jira" "io" "reflect" "strings" "time" + "github.com/andygrunwald/go-jira" "github.com/go-kit/log" "github.com/go-kit/log/level" "github.com/pkg/errors" @@ -376,10 +376,11 @@ func handleJiraErrResponse(api string, resp *jira.Response, err error, logger lo } if resp != nil && resp.StatusCode/100 != 2 { - retry := resp.StatusCode == 500 || resp.StatusCode == 503 + retry := resp.StatusCode == 500 || resp.StatusCode == 503 || resp.StatusCode == 429 + // Sometimes go-jira consumes the body (e.g. in `Search`) and includes it in the error message; + // sometimes (e.g. in `Create`) it doesn't. Include both the error and the body, just in case. body, _ := io.ReadAll(resp.Body) - // go-jira error message is not particularly helpful, replace it - return retry, errors.Errorf("JIRA request %s returned status %s, body %q", resp.Request.URL, resp.Status, string(body)) + return retry, errors.Errorf("JIRA request %s returned status %s, error %q, body %q", resp.Request.URL, resp.Status, err, body) } return false, errors.Wrapf(err, "JIRA request %s failed", api) }