feat(tags): return fetch error

msaf1980 · msaf1980 · commit cf98aa50a81f · 2024-04-26T07:32:47.000+05:00
diff --git a/cmd/carbonapi/http/tags_handler.go b/cmd/carbonapi/http/tags_handler.go
@@ -13,6 +13,7 @@ import (
 	"github.com/go-graphite/carbonapi/carbonapipb"
 	"github.com/go-graphite/carbonapi/cmd/carbonapi/config"
 	utilctx "github.com/go-graphite/carbonapi/util/ctx"
+	"github.com/go-graphite/carbonapi/zipper/helper"
 	"github.com/go-graphite/carbonapi/zipper/types"
 	"github.com/lomik/zapwriter"
 	"go.uber.org/zap"
@@ -58,8 +59,7 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
 	err := r.ParseForm()
 	if err != nil {
 		logAsError = true
-		w.Header().Set("Content-Type", contentTypeJSON)
-		_, _ = w.Write([]byte{'[', ']'})
+		setError(w, accessLogDetails, err.Error(), http.StatusBadRequest, carbonapiUUID)
 		return
 	}
 
@@ -100,10 +100,9 @@ func tagHandler(w http.ResponseWriter, r *http.Request) {
 	}
 
 	// TODO(civil): Implement stats
-	if err != nil && !merry.Is(err, types.ErrNoMetricsFetched) && !merry.Is(err, types.ErrNonFatalErrors) {
-		http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
-		accessLogDetails.HTTPCode = http.StatusInternalServerError
-		accessLogDetails.Reason = err.Error()
+	if err != nil && !merry.Is(err, types.ErrNoMetricsFetched) && (!merry.Is(err, types.ErrNonFatalErrors) || config.Config.Upstreams.RequireSuccessAll) {
+		code := merry.HTTPCode(err)
+		setError(w, accessLogDetails, helper.MerryRootError(err), code, carbonapiUUID)
 		logAsError = true
 		return
 	}
@@ -116,9 +115,7 @@ func tagHandler(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, carbonapiUUID)
 		logAsError = true
 		return
 	}
diff --git a/cmd/mockbackend/carbonapi_singlebackend_all.yaml b/cmd/mockbackend/carbonapi_singlebackend_all.yaml
@@ -0,0 +1,58 @@
+listen: "localhost:8081"
+expvar:
+  enabled: true
+  pprofEnabled: false
+  listen: ""
+concurency: 1000
+notFoundStatusCode: 200
+cache:
+   type: "mem"
+   size_mb: 0
+   defaultTimeoutSec: 60
+cpus: 0
+tz: ""
+maxBatchSize: 0
+graphite:
+    host: ""
+    interval: "60s"
+    prefix: "carbon.api"
+    pattern: "{prefix}.{fqdn}"
+idleConnections: 10
+pidFile: ""
+upstreams:
+    buckets: 10
+    timeouts:
+        find: "2s"
+        render: "10s"
+        connect: "200ms"
+    concurrencyLimitPerServer: 0
+    keepAliveInterval: "30s"
+    maxIdleConnsPerHost: 100
+    requireSuccessAll: true
+    backendsv2:
+        backends:
+          -
+            groupName: "mock-001"
+            protocol: "auto"
+            lbMethod: "all"
+            maxTries: 3
+            maxBatchSize: 0
+            keepAliveInterval: "10s"
+            concurrencyLimit: 0
+            forceAttemptHTTP2: true
+            maxIdleConnsPerHost: 1000
+            timeouts:
+                find: "15000s"
+                render: "5000s"
+                connect: "200ms"
+            servers:
+                - "http://127.0.0.1:9070"
+graphite09compat: false
+expireDelaySec: 10
+logger:
+    - logger: ""
+      file: "stderr"
+      level: "debug"
+      encoding: "console"
+      encodingTime: "iso8601"
+      encodingDuration: "seconds"
diff --git a/cmd/mockbackend/testcases/tags_error/tags_error.yaml b/cmd/mockbackend/testcases/tags_error/tags_error.yaml
@@ -48,9 +48,6 @@ test:
               contentType: "application/json"
               expectedResults:
                   - tagsAutocompelete: []
-            # TODO: error check
-            # httpCode: 503
-            # contentType: "text/plain; charset=utf-8"
 
         # 503
         - endpoint: "http://127.0.0.1:8081"
@@ -61,8 +58,6 @@ test:
               contentType: "application/json"
               expectedResults:
                   - tagsAutocompelete: []
-              # httpCode: 503
-              # contentType: "text/plain; charset=utf-8"
 
 listeners:
       - address: ":9070"
diff --git a/cmd/mockbackend/testcases/tags_error_all/tags_error_all.yaml b/cmd/mockbackend/testcases/tags_error_all/tags_error_all.yaml
@@ -0,0 +1,79 @@
+version: "v1"
+test:
+    apps:
+        - name: "carbonapi"
+          binary: "./carbonapi"
+          args:
+              - "-config"
+              - "./cmd/mockbackend/carbonapi_singlebackend_all.yaml"
+              - "-exact-config"
+    queries:
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/tags/autoComplete/values?expr=tag1%3Dv1&tag=tag2"
+          expectedResponse:
+            httpCode: 200
+            contentType: "application/json"
+            expectedResults:
+              - tagsAutocompelete:
+                - "value1"
+                - "value2"
+
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/tags/autoComplete/tags?expr=tag1%3Dv1&tagPrefix=tag"
+          expectedResponse:
+            httpCode: 200
+            contentType: "application/json"
+            expectedResults:
+              - tagsAutocompelete:
+                - "tag2"
+
+        # empty
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/tags/autoComplete/values?expr=tag1%3Dv1&tag=tag3"
+          expectedResponse:
+            httpCode: 200
+            contentType: "application/json"
+            expectedResults:
+              - tagsAutocompelete: []
+
+        # timeout
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/tags/autoComplete/values?expr=tag2%3Dv1&tag=tag3"
+          expectedResponse:
+            httpCode: 503
+            contentType: "text/plain; charset=utf-8"
+            errBody: "timeout while fetching Response\n"
+
+        # 503
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/tags/autoComplete/values?expr=tag2%3Dv1&tag=tag4"
+          expectedResponse:
+              httpCode: 503
+              contentType: "text/plain; charset=utf-8"
+              errBody: "Service Unavailable\n"
+
+listeners:
+      - address: ":9070"
+        expressions:
+            "/tags/autoComplete/values?expr=tag1%3Dv1&tag=tag2":
+                tags:
+                    - "value1"
+                    - "value2"
+
+            "/tags/autoComplete/tags?expr=tag1%3Dv1&tagPrefix=tag":
+                tags:
+                    - "tag2"
+
+            "/tags/autoComplete/values?expr=tag2%3Dv1&tag=tag3":
+                replyDelayMS: 7000
+                tags:
+                    - "value3"
+                    - "value4"
+
+            "/tags/autoComplete/values?expr=tag2%3Dv1&tag=tag4":
+                code: 503
diff --git a/zipper/broadcast/broadcast_group.go b/zipper/broadcast/broadcast_group.go
@@ -846,9 +846,9 @@ func (bg *BroadcastGroup) tagEverything(ctx context.Context, isTagName bool, que
 
 	var err merry.Error
 	if result.Err != nil {
-		err = types.ErrNonFatalErrors
-		for _, e := range result.Err {
-			err = err.WithCause(e)
+		code, errors := helper.MergeHttpErrors(result.Err)
+		if len(errors) > 0 {
+			err = types.ErrNonFatalErrors.WithHTTPCode(code).WithMessage(strings.Join(errors, "\n"))
 		}
 	}
 

Original file line number	Diff line number	Diff line change
`@@ -846,9 +846,9 @@ func (bg *BroadcastGroup) tagEverything(ctx context.Context, isTagName bool, que`
`846`	`846`
`847`	`847`	`var err merry.Error`
`848`	`848`	`if result.Err != nil {`
`849`		`- err = types.ErrNonFatalErrors`
`850`		`- for _, e := range result.Err {`
`851`		`- err = err.WithCause(e)`
	`849`	`+ code, errors := helper.MergeHttpErrors(result.Err)`
	`850`	`+ if len(errors) > 0 {`
	`851`	`+ err = types.ErrNonFatalErrors.WithHTTPCode(code).WithMessage(strings.Join(errors, "\n"))`
`852`	`852`	`}`
`853`	`853`	`}`
`854`	`854`