Skip to content

Commit

Permalink
can not use errors wrap, otherwise httpgrpc error will be converted t…
Browse files Browse the repository at this point in the history
…o err, which will be considered as 5xx error later
  • Loading branch information
wenxu1024 committed Sep 18, 2023
1 parent a064bce commit 810c9dc
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 10 deletions.
6 changes: 1 addition & 5 deletions pkg/distributor/distributor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (
"google.golang.org/grpc/codes"

"github.com/go-kit/log"
"github.com/pkg/errors"
"github.com/prometheus/client_golang/prometheus"
"github.com/prometheus/client_golang/prometheus/testutil"
"github.com/prometheus/common/model"
Expand Down Expand Up @@ -284,10 +283,7 @@ func TestDistributor_Push(t *testing.T) {
request := makeWriteRequest(tc.samples.startTimestampMs, tc.samples.num, tc.metadata)
response, err := ds[0].Push(ctx, request)
assert.Equal(t, tc.expectedResponse, response)
if err != nil {
err = errors.Cause(err)
}
assert.Equal(t, tc.expectedError, err)
assert.Equal(t, status.Code(tc.expectedError), status.Code(err))

// Check tracked Prometheus metrics. Since the Push() response is sent as soon as the quorum
// is reached, when we reach this point the 3rd ingester may not have received series/metadata
Expand Down
10 changes: 5 additions & 5 deletions pkg/ring/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ package ring
import (
"context"
"fmt"
"github.com/cortexproject/cortex/pkg/util/httpgrpcutil"
"sync"

"google.golang.org/grpc/status"

"github.com/pkg/errors"
"go.uber.org/atomic"
)

Expand Down Expand Up @@ -152,7 +152,7 @@ func (b *batchTracker) record(instance instance, err error) {
if err != nil {
// Track the number of errors by error family, and if it exceeds maxFailures
// shortcut the waiting rpc.
wrappedErr := errors.Wrapf(err, "addr=%s state=%s zone=%s", instance.desc.Addr, instance.desc.State, instance.desc.Zone)
wrappedErr := httpgrpcutil.WrapHttpGrpcError(err, "addr=%s state=%s zone=%s", instance.desc.Addr, instance.desc.State, instance.desc.Zone)
errCount := instance.itemTrackers[i].recordError(wrappedErr)
// We should return an error if we reach the maxFailure (quorum) on a given error family OR
// we dont have any remaining ingesters to try
Expand All @@ -162,13 +162,13 @@ func (b *batchTracker) record(instance instance, err error) {
// Ex: 5xx, _, 5xx -> return 5xx
if errCount > int32(sampleTrackers[i].maxFailures) {
if b.rpcsFailed.Inc() == 1 {
b.err <- errors.Wrap(sampleTrackers[i].getError(), "maxFailure (quorum) on a given error family")
b.err <- httpgrpcutil.WrapHttpGrpcError(sampleTrackers[i].getError(), "maxFailure (quorum) on a given error family")
}
continue
}
if sampleTrackers[i].remaining.Dec() == 0 {
if b.rpcsFailed.Inc() == 1 {
b.err <- errors.Wrap(sampleTrackers[i].getError(), "not enough remaining instances to try")
b.err <- httpgrpcutil.WrapHttpGrpcError(sampleTrackers[i].getError(), "not enough remaining instances to try")
}
continue
}
Expand All @@ -187,7 +187,7 @@ func (b *batchTracker) record(instance instance, err error) {
// Ex: 4xx, 5xx, 2xx
if sampleTrackers[i].remaining.Dec() == 0 {
if b.rpcsFailed.Inc() == 1 {
b.err <- errors.Wrap(sampleTrackers[i].getError(), "not enough remaining instances to try")
b.err <- httpgrpcutil.WrapHttpGrpcError(sampleTrackers[i].getError(), "not enough remaining instances to try")
}
}
}
Expand Down
23 changes: 23 additions & 0 deletions pkg/util/httpgrpcutil/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package httpgrpcutil

import (
"fmt"
"github.com/weaveworks/common/httpgrpc"
"net/http"
)

func WrapHttpGrpcError(err error, format string, args ...interface{}) error {
if err == nil {
return nil
}
msg := fmt.Sprintf(format, args...)
resp, ok := httpgrpc.HTTPResponseFromError(err)
if !ok {
return httpgrpc.Errorf(http.StatusInternalServerError, "%s, %s", msg, err)
}
return httpgrpc.ErrorFromHTTPResponse(&httpgrpc.HTTPResponse{
Code: resp.Code,
Headers: resp.Headers,
Body: []byte(fmt.Sprintf("%s, %s", msg, err)),
})
}

0 comments on commit 810c9dc

Please sign in to comment.