Skip to content

Commit

Permalink
fixup! feat(render): return error on partial targets fetch
Browse files Browse the repository at this point in the history
  • Loading branch information
msaf1980 committed Mar 13, 2024
1 parent e35ef4e commit 35974c8
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 31 deletions.
22 changes: 12 additions & 10 deletions cmd/mockbackend/e2etesting.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,14 +212,6 @@ func doTest(logger *zap.Logger, t *Query) []error {
return failures
}

if resp.StatusCode != t.ExpectedResponse.HttpCode {
failures = append(failures, merry2.Errorf("unexpected status code, got %v, expected %v",
resp.StatusCode,
t.ExpectedResponse.HttpCode,
),
)
}

contentType = resp.Header.Get("Content-Type")
if t.ExpectedResponse.ContentType != contentType {
failures = append(failures,
Expand All @@ -237,6 +229,14 @@ func doTest(logger *zap.Logger, t *Query) []error {
return failures
}

if resp.StatusCode != t.ExpectedResponse.HttpCode {
failures = append(failures, merry2.Errorf("unexpected status code, got %v, expected %v",
resp.StatusCode,
t.ExpectedResponse.HttpCode,
),
)
}

// We don't need to actually check body of response if we expect any sort of error (4xx/5xx)
if t.ExpectedResponse.HttpCode >= 300 {
return failures
Expand All @@ -256,7 +256,7 @@ func doTest(logger *zap.Logger, t *Query) []error {
}
if !sha256matched {
encodedBody := base64.StdEncoding.EncodeToString(b)
failures = append(failures, merry2.Errorf("sha256 mismatch, got '%v', expected '%v', encodedBodyy: '%v'", hashStr, t.ExpectedResponse.ExpectedResults[0].SHA256, encodedBody))
failures = append(failures, merry2.Errorf("sha256 mismatch, got '%v', expected '%v', encodedBody: '%v'", hashStr, t.ExpectedResponse.ExpectedResults[0].SHA256, encodedBody))
return failures
}
case "application/json":
Expand Down Expand Up @@ -323,7 +323,9 @@ func doTest(logger *zap.Logger, t *Query) []error {
}

default:
failures = append(failures, merry2.Errorf("unsupported content-type: got '%v'", contentType))
if resp.StatusCode == http.StatusOK {
failures = append(failures, merry2.Errorf("unsupported content-type: got '%v'", contentType))
}
}

return failures
Expand Down
18 changes: 10 additions & 8 deletions cmd/mockbackend/find.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,7 @@ func (cfg *listener) findHandler(wr http.ResponseWriter, req *http.Request) {
return
}

query := req.Form["query"]

if len(query) == 0 {
logger.Error("Bad request (no query)")
http.Error(wr, "Bad request (no query)",
http.StatusBadRequest)
return
}
var query []string

if format == protoV3Format {
body, err := io.ReadAll(req.Body)
Expand All @@ -70,6 +63,15 @@ func (cfg *listener) findHandler(wr http.ResponseWriter, req *http.Request) {
_ = pv3Request.Unmarshal(body)

query = pv3Request.Metrics
} else {
query = req.Form["query"]
}

if len(query) == 0 {
logger.Error("Bad request (no query)")
http.Error(wr, "Bad request (no query)",
http.StatusBadRequest)
return
}

logger.Info("request details",
Expand Down
27 changes: 18 additions & 9 deletions zipper/broadcast/broadcast_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,12 +307,14 @@ func (bg *BroadcastGroup) doSingleFetch(ctx context.Context, logger *zap.Logger,

// TODO(Civil): migrate limiter to merry
requests, splitErr := bg.splitRequest(ctx, request, backend)
if len(requests) == 0 && splitErr != nil {
response := types.NewServerFetchResponse()
response.Server = backend.Name()
response.AddError(splitErr)
resCh <- response
return
if len(requests) == 0 {
if splitErr != nil {
response := types.NewServerFetchResponse()
response.Server = backend.Name()
response.AddError(splitErr)
resCh <- response
return
}
}

logger = logger.With(zap.String("backend_name", backend.Name()))
Expand Down Expand Up @@ -510,7 +512,7 @@ func (bg *BroadcastGroup) Fetch(ctx context.Context, request *protov3.MultiFetch
)
}

if len(result.Response.Metrics) == 0 {
if len(result.Response.Metrics) == 0 || (bg.requireSuccessAll && len(result.Err) > 0) {
code, errors := helper.MergeHttpErrors(result.Err)
if len(errors) > 0 {
err := types.ErrFailedToFetch.WithHTTPCode(code).WithMessage(strings.Join(errors, "\n"))
Expand All @@ -537,7 +539,7 @@ func (bg *BroadcastGroup) Fetch(ctx context.Context, request *protov3.MultiFetch
)

var err merry.Error
if result.Err != nil && len(result.Err) > 0 {
if len(result.Err) > 0 {
if bg.requireSuccessAll {
code, errors := helper.MergeHttpErrors(result.Err)
if len(errors) > 0 {
Expand Down Expand Up @@ -626,7 +628,7 @@ func (bg *BroadcastGroup) Find(ctx context.Context, request *protov3.MultiGlobRe
}

var err merry.Error
if result.Err != nil && len(result.Err) > 0 {
if len(result.Response.Metrics) == 0 || (bg.requireSuccessAll && len(result.Err) > 0) {
code, errors := helper.MergeHttpErrors(result.Err)
if len(errors) > 0 {
err = types.ErrFailedToFetch.WithHTTPCode(code).WithMessage(strings.Join(errors, "\n"))
Expand Down Expand Up @@ -654,6 +656,13 @@ func (bg *BroadcastGroup) Find(ctx context.Context, request *protov3.MultiGlobRe
result.Stats.TotalMetricsCount += uint64(len(x.Matches))
}

if result.Err != nil {
err = types.ErrNonFatalErrors
for _, e := range result.Err {
err = err.WithCause(e)
}
}

return result.Response, result.Stats, err
}

Expand Down
10 changes: 6 additions & 4 deletions zipper/types/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
)

// type Fetcher func(ctx context.Context, logger *zap.Logger, client types.BackendServer, reqs interface{}, resCh chan<- types.ServerFetchResponse) {
//type Fetcher func(ctx context.Context, logger *zap.Logger, client BackendServer, reqs interface{}, resCh chan ServerFetchResponse) {
// type Fetcher func(ctx context.Context, logger *zap.Logger, client BackendServer, reqs interface{}, resCh chan ServerFetchResponse) {
type Fetcher func(ctx context.Context, logger *zap.Logger, client BackendServer, reqs interface{}, resCh chan ServerFetcherResponse)

type ServerFetcherResponse interface {
Expand Down Expand Up @@ -52,9 +52,11 @@ GATHER:
select {
case res := <-resCh:
answeredServers[res.GetServer()] = struct{}{}
_ = result.MergeI(res)
responseCount++

if err := result.MergeI(res); err == nil {
responseCount++
} else {
result.AddError(err)
}
case <-ctx.Done():
err := ErrTimeoutExceeded.WithValue("timedout_backends", NoAnswerBackends(clients, answeredServers))
result.AddError(err)
Expand Down

0 comments on commit 35974c8

Please sign in to comment.