diff --git a/cmd/carbonapi/http/find_handlers.go b/cmd/carbonapi/http/find_handlers.go index 298cae2c4..b84d318e0 100644 --- a/cmd/carbonapi/http/find_handlers.go +++ b/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 diff --git a/cmd/carbonapi/http/helper.go b/cmd/carbonapi/http/helper.go index d35f5a56b..a858965a2 100644 --- a/cmd/carbonapi/http/helper.go +++ b/cmd/carbonapi/http/helper.go @@ -2,6 +2,7 @@ package http import ( "fmt" + "html" "net/http" "strings" "time" @@ -229,6 +230,27 @@ func splitRemoteAddr(addr string) (string, string) { return tmp[0], tmp[1] } +func stripKey(key string, n int) string { + if len(key) > n+3 { + key = key[:n/2] + "..." + key[n/2+1:] + } + return key +} + +// stripError for strip network errors (ip and other private info) +func stripError(err string) string { + if strings.Contains(err, "connection refused") { + return "connection refused" + } else if strings.Contains(err, " lookup ") { + return "lookup error" + } else if strings.Contains(err, "broken pipe") { + return "broken pipe" + } else if strings.Contains(err, " connection reset ") { + return "connection reset" + } + return html.EscapeString(err) +} + 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 +307,56 @@ 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 joinErrors(errMap map[string]string, sep string, status int) (msg, reason string) { + if len(errMap) == 0 { + msg = http.StatusText(status) + } else { + var buf, rBuf strings.Builder + buf.Grow(512) + rBuf.Grow(512) + + // map is unsorted, can produce flapping ordered output, not critical + for k, err := range errMap { + if buf.Len() > 0 { + buf.WriteString(sep) + rBuf.WriteString(sep) + } + buf.WriteString(html.EscapeString(stripKey(k, 128))) + rBuf.WriteString(k) + buf.WriteString(": ") + rBuf.WriteString(": ") + buf.WriteString(html.EscapeString(stripError(err))) + rBuf.WriteString(err) + } + + msg = buf.String() + reason = rBuf.String() + } + return +} + +func setErrors(w http.ResponseWriter, accessLogDetails *carbonapipb.AccessLogDetails, errMamp map[string]string, status int, carbonapiUUID string) { + w.Header().Set(ctxHeaderUUID, carbonapiUUID) + var msg string + if status != http.StatusOK { + if len(errMamp) == 0 { + msg = http.StatusText(status) + accessLogDetails.Reason = msg + } else { + msg, accessLogDetails.Reason = joinErrors(errMamp, "\r\n", status) + } + } + accessLogDetails.HTTPCode = int32(status) + http.Error(w, msg, status) } func queryLengthLimitExceeded(query []string, maxLength uint64) bool { diff --git a/cmd/carbonapi/http/render_handler.go b/cmd/carbonapi/http/render_handler.go index f2938527c..af3b6f65e 100644 --- a/cmd/carbonapi/http/render_handler.go +++ b/cmd/carbonapi/http/render_handler.go @@ -353,16 +353,16 @@ func renderHandler(w http.ResponseWriter, r *http.Request) { // Obtain error code from the errors // In case we have only "Not Found" errors, result should be 404 // Otherwise it should be 500 - var errMsgs []string + var errMsgs map[string]string returnCode, errMsgs = helper.MergeHttpErrorMap(errors) - logger.Debug("error response or no response", zap.Strings("error", errMsgs)) + logger.Debug("error response or no response", zap.Any("error", errMsgs)) // Allow override status code for 404-not-found replies. if returnCode == http.StatusNotFound { returnCode = config.Config.NotFoundStatusCode } 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 } diff --git a/cmd/carbonapi/http/tags_handler.go b/cmd/carbonapi/http/tags_handler.go index 2663f37d3..4f3216e2c 100644 --- a/cmd/carbonapi/http/tags_handler.go +++ b/cmd/carbonapi/http/tags_handler.go @@ -21,9 +21,10 @@ import ( func tagHandler(w http.ResponseWriter, r *http.Request) { t0 := time.Now() uuid := uuid.NewV4() + carbonapiUUID := uuid.String() // TODO: Migrate to context.WithTimeout - ctx := utilctx.SetUUID(r.Context(), uuid.String()) + ctx := utilctx.SetUUID(r.Context(), carbonapiUUID) requestHeaders := utilctx.GetLogHeaders(ctx) username, _, _ := r.BasicAuth() @@ -39,7 +40,7 @@ func tagHandler(w http.ResponseWriter, r *http.Request) { var accessLogDetails = &carbonapipb.AccessLogDetails{ Handler: "tags", Username: username, - CarbonapiUUID: uuid.String(), + CarbonapiUUID: carbonapiUUID, URL: r.URL.Path, PeerIP: srcIP, PeerPort: srcPort, @@ -81,7 +82,7 @@ func tagHandler(w http.ResponseWriter, r *http.Request) { rawQuery := q.Encode() if queryLengthLimitExceeded(r.Form["query"], config.Config.MaxQueryLength) { - setError(w, accessLogDetails, "query length limit exceeded", http.StatusBadRequest, uuid.String()) + setError(w, accessLogDetails, "query length limit exceeded", http.StatusBadRequest, carbonapiUUID) logAsError = true return } @@ -123,7 +124,7 @@ func tagHandler(w http.ResponseWriter, r *http.Request) { } w.Header().Set("Content-Type", contentTypeJSON) - w.Header().Set(ctxHeaderUUID, uuid.String()) + w.Header().Set(ctxHeaderUUID, carbonapiUUID) _, _ = w.Write(b) accessLogDetails.Runtime = time.Since(t0).Seconds() accessLogDetails.HTTPCode = http.StatusOK diff --git a/cmd/mockbackend/e2etesting.go b/cmd/mockbackend/e2etesting.go index b8ab00ef3..5f99452ce 100644 --- a/cmd/mockbackend/e2etesting.go +++ b/cmd/mockbackend/e2etesting.go @@ -13,6 +13,7 @@ import ( "net/url" "os" "reflect" + "sort" "strconv" "strings" "sync" @@ -45,6 +46,8 @@ type Query struct { type ExpectedResponse struct { HttpCode int `yaml:"httpCode"` ContentType string `yaml:"contentType"` + ErrBody string `yaml:"errBody"` + ErrSort bool `yaml:"errSort"` ExpectedResults []ExpectedResult `yaml:"expectedResults"` } @@ -245,8 +248,27 @@ 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 != "" { + errStr := string(b) + if t.ExpectedResponse.ErrSort && strings.Contains(errStr, "\r\n") { + // resort error string + lastEndline := false + if strings.HasSuffix(errStr, "\n") && !strings.HasSuffix(errStr, "\r\n") { + lastEndline = true + errStr = errStr[:len(errStr)-1] + } + errs := sort.StringSlice(strings.Split(errStr, "\r\n")) + errStr = strings.Join(errs, "\r\n") + if lastEndline { + errStr += "\n" + } + } + if t.ExpectedResponse.ErrBody != errStr { + failures = append(failures, merry2.Errorf("mismatch error body, got '%s', expected '%s'", string(b), t.ExpectedResponse.ErrBody)) + } + } return failures } diff --git a/cmd/mockbackend/testcases/connection_refused/carbonapi.yaml b/cmd/mockbackend/testcases/connection_refused/carbonapi.yaml new file mode 100644 index 000000000..e9109f72d --- /dev/null +++ b/cmd/mockbackend/testcases/connection_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" diff --git a/cmd/mockbackend/testcases/connection_refused/connection_refused.yaml b/cmd/mockbackend/testcases/connection_refused/connection_refused.yaml new file mode 100644 index 000000000..e2e257c89 --- /dev/null +++ b/cmd/mockbackend/testcases/connection_refused/connection_refused.yaml @@ -0,0 +1,72 @@ +version: "v1" +test: + apps: + - name: "carbonapi" + binary: "./carbonapi" + args: + - "-config" + - "./cmd/mockbackend/testcases/connection_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" + errSort: true + - 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" + errSort: true + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/metrics/find/?query=a&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + errBody: "connection refused\n" + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/metrics/find/?query=a&query=b&format=json" + expectedResponse: + httpCode: 503 + contentType: "text/plain; charset=utf-8" + errBody: "connection refused\n" + - endpoint: "http://127.0.0.1:8081" + type: "GET" + URL: "/tags/autoComplete/values?expr=tag2%3Dv1&tag=tag4" + expectedResponse: + httpCode: 200 + contentType: "application/json" + expectedResults: + - tagsAutocompelete: [] + # TODO: query must fail + # httpCode: 503 + # contentType: "text/plain; charset=utf-8" + # errBody: "connection refused\n" + # errSort: true + +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 diff --git a/cmd/mockbackend/testcases/find_error/find_error.yaml b/cmd/mockbackend/testcases/find_error/find_error.yaml index 53b5003f7..f165bbe57 100644 --- a/cmd/mockbackend/testcases/find_error/find_error.yaml +++ b/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" diff --git a/cmd/mockbackend/testcases/render_error_all/render_error_all.yaml b/cmd/mockbackend/testcases/render_error_all/render_error_all.yaml index d312d89d0..b8ee69c27 100644 --- a/cmd/mockbackend/testcases/render_error_all/render_error_all.yaml +++ b/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" diff --git a/cmd/mockbackend/testcases/render_error_all_rr/render_error_all_rr.yaml b/cmd/mockbackend/testcases/render_error_all_rr/render_error_all_rr.yaml index a401b99ae..2c0e64748 100644 --- a/cmd/mockbackend/testcases/render_error_all_rr/render_error_all_rr.yaml +++ b/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" diff --git a/zipper/helper/requests.go b/zipper/helper/requests.go index b31e99a13..0404e936d 100644 --- a/zipper/helper/requests.go +++ b/zipper/helper/requests.go @@ -124,6 +124,46 @@ func HttpErrorCode(err merry.Error) (code int) { return } +// for stable return code on multiply errors +func recalcCode(code, newCode int) int { + if newCode == http.StatusGatewayTimeout || newCode == http.StatusBadGateway { + // simplify code, one error type for communications errors, all we can retry + newCode = http.StatusServiceUnavailable + } + if code == 0 || code == http.StatusNotFound { + return newCode + } + + if newCode >= 400 && newCode < 500 && code >= 400 && code < 500 { + if newCode == http.StatusBadRequest { + return newCode + } else if newCode == http.StatusForbidden && code != http.StatusBadRequest { + return newCode + } + } + if newCode < code { + code = newCode + } + return code +} + +// MerryRootError strip merry error chain +func MerryRootError(err error) string { + c := merry.RootCause(err) + if c == nil { + c = err + } + return merryError(c) +} + +func merryError(err error) string { + if msg := merry.Message(err); len(msg) > 0 { + return strings.TrimRight(msg, "\n") + } else { + return err.Error() + } +} + func MergeHttpErrors(errors []merry.Error) (int, []string) { returnCode := http.StatusNotFound errMsgs := make([]string, 0) @@ -141,40 +181,37 @@ func MergeHttpErrors(errors []merry.Error) (int, []string) { code = http.StatusBadRequest } - if msg := merry.Message(c); len(msg) > 0 { - errMsgs = append(errMsgs, strings.TrimRight(msg, "\n")) - } else { - errMsgs = append(errMsgs, c.Error()) - } - - if code == http.StatusGatewayTimeout || code == http.StatusBadGateway { - // simplify code, one error type for communications errors, all we can retry - code = http.StatusServiceUnavailable - } + errMsgs = append(errMsgs, merryError(c)) - 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 - } + returnCode = recalcCode(returnCode, code) } return returnCode, errMsgs } -func MergeHttpErrorMap(errorsMap map[string]merry.Error) (int, []string) { - errors := make([]merry.Error, len(errorsMap)) - i := 0 - for _, err := range errorsMap { - errors[i] = err - i++ +func MergeHttpErrorMap(errorsMap map[string]merry.Error) (returnCode int, errMap map[string]string) { + returnCode = http.StatusNotFound + errMap = make(map[string]string) + for key, err := range errorsMap { + 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 + } + + msg := merryError(c) + errMap[key] = msg + returnCode = recalcCode(returnCode, code) } - return MergeHttpErrors(errors) + return } func HttpErrorByCode(err merry.Error) merry.Error { @@ -332,7 +369,7 @@ func (c *HttpQuery) doRequest(ctx context.Context, logger *zap.Logger, server, u return &ServerResponse{Server: server, Response: body}, nil } -func (c *HttpQuery) DoQuery(ctx context.Context, logger *zap.Logger, uri string, r types.Request) (*ServerResponse, merry.Error) { +func (c *HttpQuery) DoQuery(ctx context.Context, logger *zap.Logger, uri string, r types.Request) (resp *ServerResponse, err merry.Error) { maxTries := c.maxTries if len(c.servers) > maxTries { maxTries = len(c.servers) @@ -345,11 +382,14 @@ func (c *HttpQuery) DoQuery(ctx context.Context, logger *zap.Logger, uri string, res, err := c.doRequest(ctx, logger, server, uri, r) if err != nil { logger.Debug("have errors", - zap.Error(err), + zap.String("error", err.Error()), + zap.String("server", server), ) e = e.WithCause(err).WithHTTPCode(merry.HTTPCode(err)) code = merry.HTTPCode(err) + // TODO (msaf1980): may be metric for server failures ? + // TODO (msaf1980): may be retry policy for avoid retry bad queries ? continue } @@ -359,7 +399,7 @@ func (c *HttpQuery) DoQuery(ctx context.Context, logger *zap.Logger, uri string, return nil, types.ErrMaxTriesExceeded.WithCause(e).WithHTTPCode(code) } -func (c *HttpQuery) DoQueryToAll(ctx context.Context, logger *zap.Logger, uri string, r types.Request) ([]*ServerResponse, merry.Error) { +func (c *HttpQuery) DoQueryToAll(ctx context.Context, logger *zap.Logger, uri string, r types.Request) (resp []*ServerResponse, err merry.Error) { maxTries := c.maxTries if len(c.servers) > maxTries { maxTries = len(c.servers) diff --git a/zipper/helper/requests_test.go b/zipper/helper/requests_test.go index 0b60d7187..dd804090a 100644 --- a/zipper/helper/requests_test.go +++ b/zipper/helper/requests_test.go @@ -5,6 +5,7 @@ import ( "net" "net/http" "reflect" + "strconv" "testing" "github.com/ansel1/merry" @@ -13,8 +14,6 @@ import ( ) func TestMergeHttpErrors(t *testing.T) { - type args struct { - } tests := []struct { name string errors []merry.Error @@ -120,7 +119,7 @@ func TestMergeHttpErrors(t *testing.T) { merry.New("error").WithHTTPCode(http.StatusBadRequest), merry.New("limit").WithHTTPCode(http.StatusForbidden), }, - wantCode: http.StatusForbidden, // Last win + wantCode: http.StatusBadRequest, want: []string{"error", "limit"}, }, { @@ -129,7 +128,7 @@ func TestMergeHttpErrors(t *testing.T) { merry.New("limit").WithHTTPCode(http.StatusForbidden), merry.New("error").WithHTTPCode(http.StatusBadRequest), }, - wantCode: http.StatusBadRequest, // Last win + wantCode: http.StatusBadRequest, want: []string{"limit", "error"}, }, } @@ -146,6 +145,141 @@ func TestMergeHttpErrors(t *testing.T) { } } +func TestMergeHttpErrorMap(t *testing.T) { + tests := []struct { + name string + errors map[string]merry.Error + wantCode int + want map[string]string + }{ + { + name: "NotFound", + errors: map[string]merry.Error{}, + wantCode: http.StatusNotFound, + want: map[string]string{}, + }, + { + name: "NetErr", + errors: map[string]merry.Error{ + "a": types.ErrBackendError.WithValue("server", "test").WithCause(&net.OpError{Op: "connect", Err: fmt.Errorf("refused")}).WithHTTPCode(http.StatusServiceUnavailable), + }, + wantCode: http.StatusServiceUnavailable, + want: map[string]string{"a": "connect: refused"}, + }, + { + name: "NetErr (incapsulated)", + errors: map[string]merry.Error{ + "b": types.ErrMaxTriesExceeded.WithCause(types.ErrBackendError.WithValue("server", "test").WithCause(&net.OpError{Op: "connect", Err: fmt.Errorf("refused")})).WithHTTPCode(http.StatusServiceUnavailable), + }, + wantCode: http.StatusServiceUnavailable, + want: map[string]string{"b": "connect: refused"}, + }, + { + name: "ServiceUnavailable", + errors: map[string]merry.Error{ + "d": merry.New("unavaliable").WithHTTPCode(http.StatusServiceUnavailable), + }, + wantCode: http.StatusServiceUnavailable, + want: map[string]string{"d": "unavaliable"}, + }, + { + name: "GatewayTimeout and ServiceUnavailable", + errors: map[string]merry.Error{ + "a": merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + "de": merry.New("unavaliable").WithHTTPCode(http.StatusServiceUnavailable), + }, + wantCode: http.StatusServiceUnavailable, + want: map[string]string{"a": "timeout", "de": "unavaliable"}, + }, + { + name: "ServiceUnavailable and GatewayTimeout", + errors: map[string]merry.Error{ + "de": merry.New("unavaliable").WithHTTPCode(http.StatusServiceUnavailable), + "a": merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + }, + wantCode: http.StatusServiceUnavailable, + want: map[string]string{"a": "timeout", "de": "unavaliable"}, + }, + { + name: "Forbidden and GatewayTimeout", + errors: map[string]merry.Error{ + "de": merry.New("limit").WithHTTPCode(http.StatusForbidden), + "c": merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + }, + wantCode: http.StatusForbidden, + want: map[string]string{"c": "timeout", "de": "limit"}, + }, + { + name: "GatewayTimeout and Forbidden", + errors: map[string]merry.Error{ + "a": merry.New("limit").WithHTTPCode(http.StatusForbidden), + "c": merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + }, + wantCode: http.StatusForbidden, + want: map[string]string{"a": "limit", "c": "timeout"}, + }, + { + name: "InternalServerError and Forbidden", + errors: map[string]merry.Error{ + "a": merry.New("error").WithHTTPCode(http.StatusInternalServerError), + "cd": merry.New("limit").WithHTTPCode(http.StatusForbidden), + }, + wantCode: http.StatusForbidden, + want: map[string]string{"a": "error", "cd": "limit"}, + }, + { + name: "InternalServerError and GatewayTimeout", + errors: map[string]merry.Error{ + "a": merry.New("error").WithHTTPCode(http.StatusInternalServerError), + "b": merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + }, + wantCode: http.StatusInternalServerError, + want: map[string]string{"a": "error", "b": "timeout"}, + }, + { + name: "GatewayTimeout and InternalServerError", + errors: map[string]merry.Error{ + "a": merry.New("timeout").WithHTTPCode(http.StatusGatewayTimeout), + "cd": merry.New("error").WithHTTPCode(http.StatusInternalServerError), + }, + wantCode: http.StatusInternalServerError, + want: map[string]string{"a": "timeout", "cd": "error"}, + }, + { + name: "BadRequest and Forbidden", + errors: map[string]merry.Error{ + "de": merry.New("error").WithHTTPCode(http.StatusBadRequest), + "a": merry.New("limit").WithHTTPCode(http.StatusForbidden), + }, + wantCode: http.StatusBadRequest, + want: map[string]string{"a": "limit", "de": "error"}, + }, + { + name: "Forbidden and BadRequest", + errors: map[string]merry.Error{ + "a": merry.New("limit").WithHTTPCode(http.StatusForbidden), + "b{c,de,klmn}.cde.d{c,de,klmn}.e{c,de,klmn}.k{c,de,klmn}.b{c,de,klmn}.cde.d{c,de,klmn}.e{c,de,klmn}.k{c,de,klmn}.e{c,de,klmn}.k{c,de,klmn}": merry.New("error").WithHTTPCode(http.StatusBadRequest), + }, + wantCode: http.StatusBadRequest, + want: map[string]string{ + "a": "limit", + "b{c,de,klmn}.cde.d{c,de,klmn}.e{c,de,klmn}.k{c,de,klmn}.b{c,de,klmn}.cde.d{c,de,klmn}.e{c,de,klmn}.k{c,de,klmn}.e{c,de,klmn}.k{c,de,klmn}": "error", + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotCode, got := MergeHttpErrorMap(tt.errors) + if gotCode != tt.wantCode { + t.Errorf("MergeHttpErrors() gotCode = %v, want %v", gotCode, tt.wantCode) + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("MergeHttpErrors() got = %v, want %v", got, tt.want) + } + }) + } +} + func Test_stripHtmlTags(t *testing.T) { tests := []struct { name string @@ -184,3 +318,27 @@ func Test_stripHtmlTags(t *testing.T) { }) } } + +func Test_recalcCode(t *testing.T) { + tests := []struct { + code int + newCode int + want int + }{ + {code: 500, newCode: 403, want: 403}, + {code: 403, newCode: 500, want: 403}, + {code: 403, newCode: 400, want: 400}, + {code: 400, newCode: 403, want: 400}, + {code: 500, newCode: 503, want: 500}, + {code: 503, newCode: 500, want: 500}, + {code: 503, newCode: 502, want: 503}, + {code: 0, newCode: 502, want: 503}, + } + for i, tt := range tests { + t.Run(strconv.Itoa(i), func(t *testing.T) { + if got := recalcCode(tt.code, tt.newCode); got != tt.want { + t.Errorf("recalcCode(%d, %d) = %d, want %d", tt.code, tt.newCode, got, tt.want) + } + }) + } +}