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]>

Author: 2 people

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

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)
 }