Revert "find,tags: return detailed (non-internal server) error"

This reverts commit 73b1eb2.

Related to #791

cmd/carbonapi/http/find_handlers.go

@@ -23,7 +23,6 @@ import (
 	"github.com/go-graphite/carbonapi/date"
 	"github.com/go-graphite/carbonapi/intervalset"
 	utilctx "github.com/go-graphite/carbonapi/util/ctx"
-	"github.com/go-graphite/carbonapi/zipper/helper"
 )
 
 // Find handler and it's helper functions
@@ -176,7 +175,6 @@ func findList(multiGlobs *pbv3.MultiGlobResponse) ([]byte, error) {
 func findHandler(w http.ResponseWriter, r *http.Request) {
 	t0 := time.Now()
 	uid := uuid.NewV4()
-	carbonapiUUID := uid.String()
 	// TODO: Migrate to context.WithTimeout
 	// ctx, _ := context.WithTimeout(context.TODO(), config.Config.ZipperTimeout)
 	ctx := utilctx.SetUUID(r.Context(), uid.String())
@@ -199,7 +197,7 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 	var accessLogDetails = carbonapipb.AccessLogDetails{
 		Handler:        "find",
 		Username:       username,
-		CarbonapiUUID:  carbonapiUUID,
+		CarbonapiUUID:  uid.String(),
 		URL:            r.URL.RequestURI(),
 		PeerIP:         srcIP,
 		PeerPort:       srcPort,
@@ -216,9 +214,10 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 	}()
 
 	if !ok || !format.ValidFindFormat() {
+		http.Error(w, "unsupported format: "+formatRaw, http.StatusBadRequest)
 		accessLogDetails.HTTPCode = http.StatusBadRequest
 		accessLogDetails.Reason = "unsupported format: " + formatRaw
-		writeErrorResponse(w, int(accessLogDetails.HTTPCode), accessLogDetails.Reason, carbonapiUUID)
+		logAsError = true
 		return
 	}
 
@@ -241,16 +240,15 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 		if err != nil {
 			accessLogDetails.HTTPCode = http.StatusBadRequest
 			accessLogDetails.Reason = "failed to parse message body: " + err.Error()
-			writeErrorResponse(w, http.StatusBadRequest, accessLogDetails.Reason, carbonapiUUID)
+			http.Error(w, "bad request (failed to parse format): "+err.Error(), http.StatusBadRequest)
 			return
 		}
 
 		err = pv3Request.Unmarshal(body)
 		if err != nil {
-			w.Header().Set(ctxHeaderUUID, carbonapiUUID)
 			accessLogDetails.HTTPCode = http.StatusBadRequest
 			accessLogDetails.Reason = "failed to parse message body: " + err.Error()
-			writeErrorResponse(w, http.StatusBadRequest, accessLogDetails.Reason, carbonapiUUID)
+			http.Error(w, "bad request (failed to parse format): "+err.Error(), http.StatusBadRequest)
 			return
 		}
 	} else {
@@ -260,9 +258,10 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if len(pv3Request.Metrics) == 0 {
+		http.Error(w, "missing parameter `query`", http.StatusBadRequest)
 		accessLogDetails.HTTPCode = http.StatusBadRequest
 		accessLogDetails.Reason = "missing parameter `query`"
-		writeErrorResponse(w, http.StatusBadRequest, accessLogDetails.Reason, carbonapiUUID)
+		logAsError = true
 		return
 	}
 
@@ -274,7 +273,7 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 		accessLogDetails.TotalMetricsCount += stats.TotalMetricsCount
 	}
 	if err != nil {
-		returnCode := merry.HTTPCode(helper.HttpErrorByCode(err))
+		returnCode := merry.HTTPCode(err)
 		if returnCode != http.StatusOK || multiGlobs == nil {
 			// Allow override status code for 404-not-found replies.
 			if returnCode == http.StatusNotFound {
@@ -284,9 +283,9 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 			if returnCode < 300 {
 				multiGlobs = &pbv3.MultiGlobResponse{Metrics: []pbv3.GlobResponse{}}
 			} else {
+				http.Error(w, http.StatusText(returnCode), returnCode)
 				accessLogDetails.HTTPCode = int32(returnCode)
 				accessLogDetails.Reason = err.Error()
-				writeErrorResponse(w, returnCode, accessLogDetails.Reason, carbonapiUUID)
 				// We don't want to log this as an error if it's something normal
 				// Normal is everything that is >= 500. So if config.Config.NotFoundStatusCode is 500 - this will be
 				// logged as error
@@ -366,13 +365,12 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if err != nil {
+		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
 		accessLogDetails.HTTPCode = http.StatusInternalServerError
 		accessLogDetails.Reason = err.Error()
-		writeErrorResponse(w, http.StatusInternalServerError, accessLogDetails.Reason, carbonapiUUID)
-
 		logAsError = true
 		return
 	}
 
-	writeResponse(w, http.StatusOK, b, format, jsonp, carbonapiUUID)
+	writeResponse(w, http.StatusOK, b, format, jsonp, uid.String())
 }

cmd/carbonapi/http/helper.go

@@ -158,11 +158,6 @@ func getFormat(r *http.Request, defaultFormat responseFormat) (responseFormat, b
 	return f, ok, format
 }
 
-func writeErrorResponse(w http.ResponseWriter, returnCode int, err, carbonapiUUID string) {
-	w.Header().Set(ctxHeaderUUID, carbonapiUUID)
-	http.Error(w, err, returnCode)
-}
-
 func writeResponse(w http.ResponseWriter, returnCode int, b []byte, format responseFormat, jsonp, carbonapiUUID string) {
 	//TODO: Simplify that switch
 	w.Header().Set(ctxHeaderUUID, carbonapiUUID)

cmd/carbonapi/http/tags_handler.go

@@ -20,16 +20,15 @@ import (
 
 func tagHandler(w http.ResponseWriter, r *http.Request) {
 	t0 := time.Now()
-	uid := uuid.NewV4()
-	carbonapiUUID := uid.String()
+	uuid := uuid.NewV4()
 
 	// TODO: Migrate to context.WithTimeout
-	ctx := utilctx.SetUUID(r.Context(), uid.String())
+	ctx := utilctx.SetUUID(r.Context(), uuid.String())
 	requestHeaders := utilctx.GetLogHeaders(ctx)
 	username, _, _ := r.BasicAuth()
 
 	logger := zapwriter.Logger("tag").With(
-		zap.String("carbonapi_uuid", uid.String()),
+		zap.String("carbonapi_uuid", uuid.String()),
 		zap.String("username", username),
 		zap.Any("request_headers", requestHeaders),
 	)
@@ -40,7 +39,7 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
 	var accessLogDetails = &carbonapipb.AccessLogDetails{
 		Handler:        "tags",
 		Username:       username,
-		CarbonapiUUID:  carbonapiUUID,
+		CarbonapiUUID:  uuid.String(),
 		URL:            r.URL.Path,
 		PeerIP:         srcIP,
 		PeerPort:       srcPort,
@@ -58,8 +57,8 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
 	err := r.ParseForm()
 	if err != nil {
 		logAsError = true
-		accessLogDetails.HTTPCode = int32(http.StatusBadRequest)
-		writeErrorResponse(w, http.StatusBadRequest, accessLogDetails.Reason, carbonapiUUID)
+		w.Header().Set("Content-Type", contentTypeJSON)
+		_, _ = w.Write([]byte{'[', ']'})
 		return
 	}
 
@@ -88,16 +87,15 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
 	} else if strings.HasSuffix(r.URL.Path, "values") || strings.HasSuffix(r.URL.Path, "values/") {
 		res, err = config.Config.ZipperInstance.TagValues(ctx, rawQuery, limit)
 	} else {
-		writeErrorResponse(w, http.StatusNotFound, http.StatusText(http.StatusNotFound), carbonapiUUID)
+		http.Error(w, http.StatusText(http.StatusNotFound), http.StatusNotFound)
 		accessLogDetails.HTTPCode = http.StatusNotFound
 		return
 	}
 
 	// TODO(civil): Implement stats
 	if err != nil && !merry.Is(err, types.ErrNoMetricsFetched) && !merry.Is(err, types.ErrNonFatalErrors) {
-		code := merry.HTTPCode(err)
-		writeErrorResponse(w, code, http.StatusText(code), carbonapiUUID)
-		accessLogDetails.HTTPCode = int32(code)
+		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
+		accessLogDetails.HTTPCode = http.StatusInternalServerError
 		accessLogDetails.Reason = err.Error()
 		logAsError = true
 		return
@@ -111,15 +109,15 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if err != nil {
-		writeErrorResponse(w, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError), carbonapiUUID)
+		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
 		accessLogDetails.HTTPCode = http.StatusInternalServerError
 		accessLogDetails.Reason = err.Error()
 		logAsError = true
 		return
 	}
 
 	w.Header().Set("Content-Type", contentTypeJSON)
-	w.Header().Set(ctxHeaderUUID, uid.String())
+	w.Header().Set(ctxHeaderUUID, uuid.String())
 	_, _ = w.Write(b)
 	accessLogDetails.Runtime = time.Since(t0).Seconds()
 	accessLogDetails.HTTPCode = http.StatusOK

zipper/broadcast/broadcast_group.go

@@ -607,8 +607,7 @@ func (bg *BroadcastGroup) Find(ctx context.Context, request *protov3.MultiGlobRe
 	if len(result.Response.Metrics) == 0 {
 		nonNotFoundErrors := types.ReturnNonNotFoundError(result.Err)
 		if nonNotFoundErrors != nil {
-			code := helper.MergeHttpErrorsCode(result.Err)
-			err := types.ErrFailedToFetch.WithHTTPCode(code)
+			err := types.ErrFailedToFetch.WithHTTPCode(500)
 			for _, e := range nonNotFoundErrors {
 				err = err.WithCause(e)
 			}
@@ -819,8 +818,10 @@ func (bg *BroadcastGroup) tagEverything(ctx context.Context, isTagName bool, que
 
 	var err merry.Error
 	if result.Err != nil {
-		code := helper.MergeHttpErrorsCode(result.Err)
-		err = types.ErrFailedToFetch.WithHTTPCode(code)
+		err = types.ErrNonFatalErrors
+		for _, e := range result.Err {
+			err = err.WithCause(e)
+		}
 	}
 
 	return result.Response, err

zipper/helper/requests.go

@@ -98,41 +98,6 @@ func requestError(err error, server string) merry.Error {
 	return types.ErrResponceError.WithValue("server", server)
 }
 
-func MergeHttpErrorsCode(errors []merry.Error) (returnCode int) {
-	returnCode = http.StatusNotFound
-	for _, err := range errors {
-		c := merry.RootCause(err)
-		if c == nil {
-			c = err
-		}
-
-		code := merry.HTTPCode(err)
-		if code == http.StatusNotFound {
-			continue
-		} else if code == http.StatusInternalServerError && merry.Is(c, parser.ErrInvalidArg) {
-			// check for invalid args, see applyByNode rewrite function
-			code = http.StatusBadRequest
-		}
-
-		if code == http.StatusGatewayTimeout || code == http.StatusBadGateway {
-			// simplify code, one error type for communications errors, all we can retry
-			code = http.StatusServiceUnavailable
-		}
-
-		if code == http.StatusBadRequest {
-			// The 400 is returned on wrong requests, e.g. non-existent functions
-			returnCode = code
-		} else if returnCode == http.StatusNotFound || code == http.StatusForbidden {
-			// First error or access denied (may be limits or other)
-			returnCode = code
-		} else if code != http.StatusServiceUnavailable {
-			returnCode = code
-		}
-	}
-
-	return returnCode
-}
-
 func MergeHttpErrors(errors []merry.Error) (int, []string) {
 	returnCode := http.StatusNotFound
 	errMsgs := make([]string, 0)

zipper/zipper.go

@@ -161,6 +161,7 @@ func NewZipper(sender func(*types.Stats), cfg *config.Config, logger *zap.Logger
 		)
 	}
 
+
 	logger.Error("DEBUG ERROR LOGGGGG", zap.Any("cfg", cfg))
 	broadcastGroup, err := broadcast.New(
 		broadcast.WithLogger(logger),