fix(find): return detailed error instead of 500

cmd/carbonapi/http/find_handlers.go

@@ -214,9 +214,7 @@ 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
+		setError(w, &accessLogDetails, "unsupported format: "+formatRaw, http.StatusBadRequest, uid.String())
 		logAsError = true
 		return
 	}
@@ -238,17 +236,15 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 	if format == protoV3Format {
 		body, err := io.ReadAll(r.Body)
 		if err != nil {
-			accessLogDetails.HTTPCode = http.StatusBadRequest
-			accessLogDetails.Reason = "failed to parse message body: " + err.Error()
-			http.Error(w, "bad request (failed to parse format): "+err.Error(), http.StatusBadRequest)
+			setError(w, &accessLogDetails, "failed to parse message body: "+err.Error(), http.StatusBadRequest, uid.String())
+			logAsError = true
 			return
 		}
 
 		err = pv3Request.Unmarshal(body)
 		if err != nil {
-			accessLogDetails.HTTPCode = http.StatusBadRequest
-			accessLogDetails.Reason = "failed to parse message body: " + err.Error()
-			http.Error(w, "bad request (failed to parse format): "+err.Error(), http.StatusBadRequest)
+			setError(w, &accessLogDetails, "failed to parse message body: "+err.Error(), http.StatusBadRequest, uid.String())
+			logAsError = true
 			return
 		}
 	} else {
@@ -258,9 +254,7 @@ 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`"
+		setError(w, &accessLogDetails, "missing parameter `query`", http.StatusBadRequest, uid.String())
 		logAsError = true
 		return
 	}
@@ -283,9 +277,7 @@ 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()
+				setError(w, &accessLogDetails, http.StatusText(returnCode), returnCode, uid.String())
 				// 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
@@ -365,9 +357,7 @@ 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()
+		setError(w, &accessLogDetails, err.Error(), http.StatusInternalServerError, uid.String())
 		logAsError = true
 		return
 	}

cmd/mockbackend/e2etesting.go

@@ -12,6 +12,7 @@ import (
 	"net/http"
 	"net/url"
 	"os"
+	"reflect"
 	"strconv"
 	"strings"
 	"sync"
@@ -48,11 +49,21 @@ type ExpectedResponse struct {
 }
 
 type ExpectedResult struct {
-	SHA256  []string `yaml:"sha256"`
-	Metrics []CarbonAPIResponse
+	SHA256      []string `yaml:"sha256"`
+	Metrics     []RenderResponse
+	MetricsFind []MetricsFindResponse `json:"metricsFind" yaml:"metricsFind"`
 }
 
-type CarbonAPIResponse struct {
+type MetricsFindResponse struct {
+	AllowChildren int               `json:"allowChildren" yaml:"allowChildren"`
+	Expandable    int               `json:"expandable" yaml:"expandable"`
+	Leaf          int               `json:"leaf" yaml:"leaf"`
+	Id            string            `json:"id" yaml:"id"`
+	Text          string            `json:"text" yaml:"text"`
+	Context       map[string]string `json:"context" yaml:"context"`
+}
+
+type RenderResponse struct {
 	Target     string            `json:"target" yaml:"target"`
 	Datapoints []Datapoint       `json:"datapoints" yaml:"datapoints"`
 	Tags       map[string]string `json:"tags" yaml:"tags"`
@@ -121,7 +132,7 @@ func (d *Datapoint) UnmarshalYAML(unmarshal func(interface{}) error) error {
 	return nil
 }
 
-func isMetricsEqual(m1, m2 CarbonAPIResponse) error {
+func isRenderEqual(m1, m2 RenderResponse) error {
 	if m1.Target != m2.Target {
 		return fmt.Errorf("target mismatch, got '%v', expected '%v'", m1.Target, m2.Target)
 	}
@@ -249,30 +260,65 @@ func doTest(logger *zap.Logger, t *Query) []error {
 			return failures
 		}
 	case "application/json":
-		res := make([]CarbonAPIResponse, 0, 1)
-		err := json.Unmarshal(b, &res)
-		if err != nil {
-			err = merry2.Prepend(err, "failed to parse response")
-			failures = append(failures, err)
-			return failures
-		}
+		if strings.HasPrefix(t.URL, "/metrics/find") {
+			res := make([]MetricsFindResponse, 0, 1)
+			err := json.Unmarshal(b, &res)
+			if err != nil {
+				err = merry2.Prepend(err, "failed to parse response")
+				failures = append(failures, err)
+				return failures
+			}
 
-		if len(t.ExpectedResponse.ExpectedResults) == 0 {
-			return failures
-		}
+			if len(t.ExpectedResponse.ExpectedResults) == 0 {
+				if len(res) > 0 {
+					failures = append(failures, merry2.Errorf("unexpected amount of results, got %v, expected 0", len(res)))
+				}
+				return failures
+			}
 
-		if len(res) != len(t.ExpectedResponse.ExpectedResults[0].Metrics) {
-			failures = append(failures, merry2.Errorf("unexpected amount of results, got %v, expected %v",
-				len(res),
-				len(t.ExpectedResponse.ExpectedResults[0].Metrics)))
-			return failures
-		}
+			if len(res) != len(t.ExpectedResponse.ExpectedResults[0].MetricsFind) {
+				failures = append(failures, merry2.Errorf("unexpected amount of results, got %v, expected %v",
+					len(res),
+					len(t.ExpectedResponse.ExpectedResults[0].MetricsFind)))
+				return failures
+			}
 
-		for i := range res {
-			err := isMetricsEqual(res[i], t.ExpectedResponse.ExpectedResults[0].Metrics[i])
+			for i := range res {
+				if !reflect.DeepEqual(res[i], t.ExpectedResponse.ExpectedResults[0].MetricsFind[i]) {
+					err = fmt.Errorf("metrics find[%d] are not equal, got=`%+v`, expected=`%+v`", i, res[i], t.ExpectedResponse.ExpectedResults[0].MetricsFind[i])
+					failures = append(failures, err)
+				}
+			}
+		} else {
+			// render
+			res := make([]RenderResponse, 0, 1)
+			err := json.Unmarshal(b, &res)
 			if err != nil {
-				err = merry2.Prependf(err, "metrics are not equal, got=`%+v`, expected=`%+v`", res[i], t.ExpectedResponse.ExpectedResults[0].Metrics[i])
+				err = merry2.Prepend(err, "failed to parse response")
 				failures = append(failures, err)
+				return failures
+			}
+
+			if len(t.ExpectedResponse.ExpectedResults) == 0 {
+				if len(res) > 0 {
+					failures = append(failures, merry2.Errorf("unexpected amount of results, got %v, expected 0", len(res)))
+				}
+				return failures
+			}
+
+			if len(res) != len(t.ExpectedResponse.ExpectedResults[0].Metrics) {
+				failures = append(failures, merry2.Errorf("unexpected amount of results, got %v, expected %v",
+					len(res),
+					len(t.ExpectedResponse.ExpectedResults[0].Metrics)))
+				return failures
+			}
+
+			for i := range res {
+				err := isRenderEqual(res[i], t.ExpectedResponse.ExpectedResults[0].Metrics[i])
+				if err != nil {
+					err = merry2.Prependf(err, "metrics are not equal, got=`%+v`, expected=`%+v`", res[i], t.ExpectedResponse.ExpectedResults[0].Metrics[i])
+					failures = append(failures, err)
+				}
 			}
 		}
 

cmd/mockbackend/find.go

@@ -48,6 +48,13 @@ func (cfg *listener) findHandler(wr http.ResponseWriter, req *http.Request) {
 
 	query := req.Form["query"]
 
+	if len(query) == 0 {
+		logger.Error("Bad request (no query)")
+		http.Error(wr, "Bad request (no query)",
+			http.StatusBadRequest)
+		return
+	}
+
 	if format == protoV3Format {
 		body, err := io.ReadAll(req.Body)
 		if err != nil {
@@ -56,6 +63,7 @@ func (cfg *listener) findHandler(wr http.ResponseWriter, req *http.Request) {
 			)
 			http.Error(wr, "Bad request (unsupported format)",
 				http.StatusBadRequest)
+			return
 		}
 
 		var pv3Request carbonapi_v3_pb.MultiGlobRequest
@@ -72,23 +80,7 @@ func (cfg *listener) findHandler(wr http.ResponseWriter, req *http.Request) {
 		Metrics: []carbonapi_v3_pb.GlobResponse{},
 	}
 
-	if query[0] != "*" {
-		for m := range cfg.Listener.Expressions {
-			globMatches := []carbonapi_v3_pb.GlobMatch{}
-
-			for _, metric := range cfg.Expressions[m].Data {
-				globMatches = append(globMatches, carbonapi_v3_pb.GlobMatch{
-					Path:   metric.MetricName,
-					IsLeaf: true,
-				})
-			}
-			multiGlobs.Metrics = append(multiGlobs.Metrics,
-				carbonapi_v3_pb.GlobResponse{
-					Name:    cfg.Expressions[m].PathExpression,
-					Matches: globMatches,
-				})
-		}
-	} else {
+	if query[0] == "*" {
 		returnMap := make(map[string]struct{})
 		for m := range cfg.Listener.Expressions {
 			response := cfg.Expressions[m]
@@ -115,6 +107,33 @@ func (cfg *listener) findHandler(wr http.ResponseWriter, req *http.Request) {
 				Name:    "*",
 				Matches: globMatches,
 			})
+	} else {
+		for _, m := range query {
+			globMatches := []carbonapi_v3_pb.GlobMatch{}
+			if response, ok := cfg.Expressions[m]; ok {
+				if response.ReplyDelayMS > 0 {
+					delay := time.Duration(response.ReplyDelayMS) * time.Millisecond
+					time.Sleep(delay)
+				}
+				if response.Code != 0 && response.Code != http.StatusOK {
+					// return first error
+					http.Error(wr, http.StatusText(response.Code), response.Code)
+					return
+				}
+
+				for _, metric := range cfg.Expressions[m].Data {
+					globMatches = append(globMatches, carbonapi_v3_pb.GlobMatch{
+						Path:   metric.MetricName,
+						IsLeaf: true,
+					})
+				}
+				multiGlobs.Metrics = append(multiGlobs.Metrics,
+					carbonapi_v3_pb.GlobResponse{
+						Name:    cfg.Expressions[m].PathExpression,
+						Matches: globMatches,
+					})
+			}
+		}
 	}
 
 	if cfg.Listener.ShuffleResults {

cmd/mockbackend/testcases/find_error/find_error.yaml

@@ -0,0 +1,94 @@
+version: "v1"
+test:
+    apps:
+        - name: "carbonapi"
+          binary: "./carbonapi"
+          args:
+              - "-config"
+              - "./cmd/mockbackend/testcases/render_error/carbonapi.yaml"
+    queries:
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/metrics/find?query=a&format=json"
+          expectedResponse:
+              httpCode: 200
+              contentType: "application/json"
+              expectedResults:
+                  - metricsFind:
+                      - allowChildren: 0
+                        expandable: 0
+                        leaf: 1
+                        id: "a"
+                        text: "a"
+                        context: {}
+
+        # empty
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/render/?target=b&format=json"
+          expectedResponse:
+              httpCode: 200
+              contentType: "application/json"
+
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/metrics/find?query=a&query=b&format=json"
+          expectedResponse:
+              httpCode: 200
+              contentType: "application/json"
+              expectedResults:
+                  - metricsFind:
+                      - allowChildren: 0
+                        expandable: 0
+                        leaf: 1
+                        id: "a"
+                        text: "a"
+                        context: {}
+
+        # timeout
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/metrics/find?query=c&format=json"
+          expectedResponse:
+              httpCode: 503
+              contentType: "text/plain; charset=utf-8"
+
+        # 503
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/metrics/find?query=d&format=json"
+          expectedResponse:
+              httpCode: 503
+              contentType: "text/plain; charset=utf-8"
+
+        # 503, partial success
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/metrics/find?query=a&query=d&format=json"
+          expectedResponse:
+              httpCode: 503
+              contentType: "text/plain; charset=utf-8"
+
+listeners:
+      - address: ":9070"
+        expressions:
+            "a":
+                pathExpression: "a"
+                data:
+                    - metricName: "a"
+                      values: [0,1,2,2,3]
+
+            # timeout
+            "c":
+                pathExpression: "b"
+                emptyBody: true
+                code: 404
+                replyDelayMS: 7000
+                data:
+                    - metricName: "c"
+                      values: [0,1,2,2,3]
+
+            "d":
+                pathExpression: "d"
+                emptyBody: true
+                code: 503

cmd/mockbackend/testcases/render_error_all/carbonapi.yaml

@@ -42,7 +42,7 @@ upstreams:
             forceAttemptHTTP2: true
             maxIdleConnsPerHost: 1000
             timeouts:
-                find: "3s"
+                find: "600s"
                 render: "5s"
                 connect: "200ms"
             servers: