Better Jira error handling (prometheus-community#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 <[email protected]> * Also include HTTP response status 429 among retriable errors. Signed-off-by: Alin Sinpalean <[email protected]> * 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 <[email protected]> --------- Signed-off-by: Alin Sinpalean <[email protected]> Co-authored-by: Alin Sinpalean <[email protected]>
KazanExpress · May 13, 2024 · 17d1ac3 · 17d1ac3
1 parent 2201daf
commit 17d1ac3
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 6 deletions.
diff --git 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
@@ -23,7 +23,6 @@ import (
 	"time"
 
 	"github.com/andygrunwald/go-jira"
-
 	"github.com/go-kit/log"
 	"github.com/go-kit/log/level"
 	"github.com/pkg/errors"
@@ -383,10 +382,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)
 }