feat(render,find): strip private info from returned errors info

cmd/carbonapi/http/find_handlers.go

@@ -23,6 +23,7 @@ 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
@@ -283,7 +284,7 @@ func findHandler(w http.ResponseWriter, r *http.Request) {
 			if returnCode < 300 {
 				multiGlobs = &pbv3.MultiGlobResponse{Metrics: []pbv3.GlobResponse{}}
 			} else {
-				setError(w, &accessLogDetails, http.StatusText(returnCode), returnCode, uid.String())
+				setError(w, &accessLogDetails, helper.MerryRootError(err), 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

cmd/carbonapi/http/helper.go

@@ -2,6 +2,7 @@ package http
 
 import (
 	"fmt"
+	"html"
 	"net/http"
 	"strings"
 	"time"
@@ -229,6 +230,46 @@ func splitRemoteAddr(addr string) (string, string) {
 	return tmp[0], tmp[1]
 }
 
+// stripError for strip network errors (ip and other private info)
+func stripError(err string) string {
+	k, v, ok := strings.Cut(err, ": ")
+	if ok {
+		if strings.Contains(v, "connection refused") {
+			return html.EscapeString(k) + ": connection refused"
+		} else if strings.Contains(v, " lookup ") {
+			return html.EscapeString(k) + ": lookup error"
+		} else if strings.Contains(v, "broken pipe") {
+			return html.EscapeString(k) + ": broken pipe"
+		} else if strings.Contains(v, " connection reset ") {
+			return html.EscapeString(k) + ": connection reset"
+		}
+	}
+	return html.EscapeString(err)
+}
+
+func joinErrors(errs []string, sep string, status int) (msg, reason string) {
+	if len(errs) == 0 {
+		msg = http.StatusText(status)
+	} else {
+		n := len(sep) * (len(errs) - 1)
+		for i := 0; i < len(errs); i++ {
+			n += len(errs[i])
+		}
+
+		var b strings.Builder
+		b.Grow(n)
+		b.WriteString(stripError(errs[0]))
+		for _, s := range errs[1:] {
+			b.WriteString(sep)
+			b.WriteString(stripError(s))
+		}
+
+		reason = b.String()
+		msg = reason
+	}
+	return
+}
+
 func buildParseErrorString(target, e string, err error) string {
 	msg := fmt.Sprintf("%s\n\n%-20s: %s\n", http.StatusText(http.StatusBadRequest), "Target", target)
 	if err != nil {
@@ -285,9 +326,28 @@ func timestampTruncate(ts int64, duration time.Duration, durations []config.Dura
 
 func setError(w http.ResponseWriter, accessLogDetails *carbonapipb.AccessLogDetails, msg string, status int, carbonapiUUID string) {
 	w.Header().Set(ctxHeaderUUID, carbonapiUUID)
-	http.Error(w, http.StatusText(status)+": "+msg, status)
+	if msg == "" {
+		msg = http.StatusText(status)
+	}
 	accessLogDetails.Reason = msg
 	accessLogDetails.HTTPCode = int32(status)
+	msg = html.EscapeString(stripError(msg))
+	http.Error(w, msg, status)
+}
+
+func setErrors(w http.ResponseWriter, accessLogDetails *carbonapipb.AccessLogDetails, errMsgs []string, status int, carbonapiUUID string) {
+	w.Header().Set(ctxHeaderUUID, carbonapiUUID)
+	var msg string
+	if status != http.StatusOK {
+		if len(errMsgs) == 0 {
+			msg = http.StatusText(status)
+			accessLogDetails.Reason = msg
+		} else {
+			msg, accessLogDetails.Reason = joinErrors(errMsgs, "\r\n", status)
+		}
+	}
+	accessLogDetails.HTTPCode = int32(status)
+	http.Error(w, msg, status)
 }
 
 func queryLengthLimitExceeded(query []string, maxLength uint64) bool {

cmd/carbonapi/http/render_handler.go

@@ -362,7 +362,7 @@ func renderHandler(w http.ResponseWriter, r *http.Request) {
 		}
 
 		if returnCode == http.StatusBadRequest || returnCode == http.StatusNotFound || returnCode == http.StatusForbidden || returnCode >= 500 {
-			setError(w, accessLogDetails, strings.Join(errMsgs, ","), returnCode, uid.String())
+			setErrors(w, accessLogDetails, errMsgs, returnCode, uid.String())
 			logAsError = true
 			return
 		}

cmd/mockbackend/e2etesting.go

@@ -45,6 +45,7 @@ type Query struct {
 type ExpectedResponse struct {
 	HttpCode        int              `yaml:"httpCode"`
 	ContentType     string           `yaml:"contentType"`
+	ErrBody         string           `yaml:"errBody"`
 	ExpectedResults []ExpectedResult `yaml:"expectedResults"`
 }
 
@@ -245,8 +246,11 @@ func doTest(logger *zap.Logger, t *Query, verbose bool) []error {
 		)
 	}
 
-	// We don't need to actually check body of response if we expect any sort of error (4xx/5xx)
+	// We don't need to actually check body of response if we expect any sort of error (4xx/5xx), but for check error handling do this
 	if t.ExpectedResponse.HttpCode >= 300 {
+		if t.ExpectedResponse.ErrBody != "" && t.ExpectedResponse.ErrBody != string(b) {
+			failures = append(failures, merry2.Errorf("mismatch error body, got '%s', expected '%s'", string(b), t.ExpectedResponse.ErrBody))
+		}
 		return failures
 	}
 

cmd/mockbackend/testcases/find_error/find_error.yaml

@@ -61,6 +61,7 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "Service Unavailable\n"
 
         # 503, partial success
         - endpoint: "http://127.0.0.1:8081"
@@ -69,6 +70,7 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "Service Unavailable\n"
 
 listeners:
       - address: ":9070"

cmd/mockbackend/testcases/render_error_all/render_error_all.yaml

@@ -45,6 +45,7 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "c: timeout while fetching Response\n"
 
         # 503
         - endpoint: "http://127.0.0.1:8081"
@@ -53,6 +54,7 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "d: Service Unavailable\n"
 
         # partial success
         - endpoint: "http://127.0.0.1:8081"
@@ -61,6 +63,7 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "d: Service Unavailable\n"
 
         # partial success, must fail, target d failed
         - endpoint: "http://127.0.0.1:8081"
@@ -69,6 +72,7 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "divideSeries(a,d): Service Unavailable\n"
 
 listeners:
       - address: ":9070"

cmd/mockbackend/testcases/render_error_all_rr/render_error_all_rr.yaml

@@ -81,6 +81,7 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "d: Service Unavailable\n"
 
         # partial success
         - endpoint: "http://127.0.0.1:8081"
@@ -89,6 +90,7 @@ test:
           expectedResponse:
               httpCode: 503
               contentType: "text/plain; charset=utf-8"
+              errBody: "d: Service Unavailable\n"
 
         # partial success
         # TODO: must fail, target d failed
@@ -98,6 +100,7 @@ test:
           expectedResponse:
             httpCode: 503
             contentType: "text/plain; charset=utf-8"
+            errBody: "divideSeries(a,d): Service Unavailable\n"
 
 listeners:
       - address: ":9070"

cmd/mockbackend/testcases/render_error_refused/carbonapi.yaml

@@ -0,0 +1,57 @@
+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: "5s"
+        connect: "200ms"
+    concurrencyLimitPerServer: 0
+    keepAliveInterval: "30s"
+    maxIdleConnsPerHost: 100
+    backendsv2:
+        backends:
+          -
+            groupName: "mock-001"
+            protocol: "auto"
+            lbMethod: "all"
+            maxTries: 1
+            maxBatchSize: 0
+            keepAliveInterval: "10s"
+            concurrencyLimit: 0
+            forceAttemptHTTP2: true
+            maxIdleConnsPerHost: 1000
+            timeouts:
+                find: "3s"
+                render: "5s"
+                connect: "200ms"
+            servers:
+                - "http://127.0.0.1:9071"
+graphite09compat: false
+expireDelaySec: 10
+logger:
+    - logger: ""
+      file: "stderr"
+      level: "debug"
+      encoding: "console"
+      encodingTime: "iso8601"
+      encodingDuration: "seconds"

cmd/mockbackend/testcases/render_error_refused/render_error_refused.yaml

@@ -0,0 +1,43 @@
+version: "v1"
+test:
+    apps:
+        - name: "carbonapi"
+          binary: "./carbonapi"
+          args:
+              - "-config"
+              - "./cmd/mockbackend/testcases/render_error_refused/carbonapi.yaml"
+              - "-exact-config"
+    queries:
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/render/?target=a&format=json"
+          expectedResponse:
+              httpCode: 503
+              contentType: "text/plain; charset=utf-8"
+              errBody: "a: connection refused\n"
+        - endpoint: "http://127.0.0.1:8081"
+          type: "GET"
+          URL: "/render/?target=a&target=b&format=json"
+          expectedResponse:
+              httpCode: 503
+              contentType: "text/plain; charset=utf-8"
+              errBody: "a: connection refused\r\nb: connection refused\n"
+
+listeners:
+      - address: ":9070"
+        expressions:
+            "a":
+                pathExpression: "a"
+                data:
+                    - metricName: "a"
+                      values: [0,1,2,2,3]
+
+            # timeout
+            "c":
+                pathExpression: "c"
+                code: 404
+                replyDelayMS: 7000
+
+            "d":
+                pathExpression: "d"
+                code: 503