Skip to content

Commit

Permalink
Merge pull request #826 from msaf1980/fix/tags_error_code
Browse files Browse the repository at this point in the history
tags/autoComplete: return detailed error code instead of 500
  • Loading branch information
msaf1980 authored May 30, 2024
2 parents 0b081ee + df67ce0 commit c2229ea
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 17 deletions.
15 changes: 6 additions & 9 deletions cmd/carbonapi/http/tags_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down
58 changes: 58 additions & 0 deletions cmd/mockbackend/carbonapi_singlebackend_all.yaml
Original file line number Diff line number Diff line change
@@ -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"
5 changes: 0 additions & 5 deletions cmd/mockbackend/testcases/tags_error/tags_error.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -61,8 +58,6 @@ test:
contentType: "application/json"
expectedResults:
- tagsAutocompelete: []
# httpCode: 503
# contentType: "text/plain; charset=utf-8"

listeners:
- address: ":9070"
Expand Down
79 changes: 79 additions & 0 deletions cmd/mockbackend/testcases/tags_error_all/tags_error_all.yaml
Original file line number Diff line number Diff line change
@@ -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
6 changes: 3 additions & 3 deletions zipper/broadcast/broadcast_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
}

Expand Down

0 comments on commit c2229ea

Please sign in to comment.