diff --git a/pkg/block/fetcher.go b/pkg/block/fetcher.go index fd0abe3b48..c0e04dbe8e 100644 --- a/pkg/block/fetcher.go +++ b/pkg/block/fetcher.go @@ -455,7 +455,7 @@ func (f *BaseFetcher) fetch(ctx context.Context, metrics *fetcherMetrics, filter metrics.submit() if len(resp.metaErrs) > 0 { - return metas, resp.partial, errors.Wrap(resp.metaErrs, "incomplete view") + return metas, resp.partial, errors.Wrap(resp.metaErrs.Err(), "incomplete view") } level.Info(f.logger).Log("msg", "successfully synchronized block metadata", "duration", time.Since(start).String(), "cached", len(f.cached), "returned", len(metas), "partial", len(resp.partial)) diff --git a/pkg/compact/compact.go b/pkg/compact/compact.go index e27478deda..a4345319fa 100644 --- a/pkg/compact/compact.go +++ b/pkg/compact/compact.go @@ -543,7 +543,7 @@ func (e HaltError) Error() string { // IsHaltError returns true if the base error is a HaltError. // If a multierror is passed, any halt error will return true. func IsHaltError(err error) bool { - if multiErr, ok := errors.Cause(err).(errutil.MultiError); ok { + if multiErr, ok := errors.Cause(err).(errutil.NonNilMultiError); ok { for _, err := range multiErr { if _, ok := errors.Cause(err).(HaltError); ok { return true @@ -576,7 +576,7 @@ func (e RetryError) Error() string { // IsRetryError returns true if the base error is a RetryError. // If a multierror is passed, all errors must be retriable. func IsRetryError(err error) bool { - if multiErr, ok := errors.Cause(err).(errutil.MultiError); ok { + if multiErr, ok := errors.Cause(err).(errutil.NonNilMultiError); ok { for _, err := range multiErr { if _, ok := errors.Cause(err).(RetryError); !ok { return false diff --git a/pkg/compact/compact_test.go b/pkg/compact/compact_test.go index 47423f8812..4f8140e7ce 100644 --- a/pkg/compact/compact_test.go +++ b/pkg/compact/compact_test.go @@ -33,11 +33,11 @@ func TestHaltMultiError(t *testing.T) { nonHaltErr := errors.New("not a halt error") errs := errutil.MultiError{nonHaltErr} - testutil.Assert(t, !IsHaltError(errs), "should not be a halt error") + testutil.Assert(t, !IsHaltError(errs.Err()), "should not be a halt error") errs.Add(haltErr) - testutil.Assert(t, IsHaltError(errs), "if any halt errors are present this should return true") - testutil.Assert(t, IsHaltError(errors.Wrap(errs, "wrap")), "halt error with wrap") + testutil.Assert(t, IsHaltError(errs.Err()), "if any halt errors are present this should return true") + testutil.Assert(t, IsHaltError(errors.Wrap(errs.Err(), "wrap")), "halt error with wrap") } @@ -46,15 +46,15 @@ func TestRetryMultiError(t *testing.T) { nonRetryErr := errors.New("not a retry error") errs := errutil.MultiError{nonRetryErr} - testutil.Assert(t, !IsRetryError(errs), "should not be a retry error") + testutil.Assert(t, !IsRetryError(errs.Err()), "should not be a retry error") errs = errutil.MultiError{retryErr} - testutil.Assert(t, IsRetryError(errs), "if all errors are retriable this should return true") + testutil.Assert(t, IsRetryError(errs.Err()), "if all errors are retriable this should return true") - testutil.Assert(t, IsRetryError(errors.Wrap(errs, "wrap")), "retry error with wrap") + testutil.Assert(t, IsRetryError(errors.Wrap(errs.Err(), "wrap")), "retry error with wrap") errs = errutil.MultiError{nonRetryErr, retryErr} - testutil.Assert(t, !IsRetryError(errs), "mixed errors should return false") + testutil.Assert(t, !IsRetryError(errs.Err()), "mixed errors should return false") } func TestRetryError(t *testing.T) { diff --git a/pkg/errutil/multierror.go b/pkg/errutil/multierror.go index e51bf4554e..aa53706dde 100644 --- a/pkg/errutil/multierror.go +++ b/pkg/errutil/multierror.go @@ -12,30 +12,12 @@ import ( // Errors used to construct it. type MultiError []error -// Returns a concatenated string of the contained errors. -func (es MultiError) Error() string { - var buf bytes.Buffer - - if len(es) > 1 { - fmt.Fprintf(&buf, "%d errors: ", len(es)) - } - - for i, err := range es { - if i != 0 { - buf.WriteString("; ") - } - buf.WriteString(err.Error()) - } - - return buf.String() -} - // Add adds the error to the error list if it is not nil. func (es *MultiError) Add(err error) { if err == nil { return } - if merr, ok := err.(MultiError); ok { + if merr, ok := err.(NonNilMultiError); ok { *es = append(*es, merr...) } else { *es = append(*es, err) @@ -47,5 +29,25 @@ func (es MultiError) Err() error { if len(es) == 0 { return nil } - return es + return NonNilMultiError(es) +} + +type NonNilMultiError MultiError + +// Returns a concatenated string of the contained errors. +func (es NonNilMultiError) Error() string { + var buf bytes.Buffer + + if len(es) > 1 { + fmt.Fprintf(&buf, "%d errors: ", len(es)) + } + + for i, err := range es { + if i != 0 { + buf.WriteString("; ") + } + buf.WriteString(err.Error()) + } + + return buf.String() } diff --git a/pkg/receive/handler.go b/pkg/receive/handler.go index 392ecabde1..0337fe495f 100644 --- a/pkg/receive/handler.go +++ b/pkg/receive/handler.go @@ -563,7 +563,7 @@ func (h *Handler) fanoutForward(pctx context.Context, tenant string, replicas ma return fctx.Err() case err, more := <-ec: if !more { - return errs + return errs.Err() } if err == nil { success++ @@ -692,7 +692,7 @@ func determineWriteErrorCause(err error, threshold int) error { } unwrappedErr := errors.Cause(err) - errs, ok := unwrappedErr.(errutil.MultiError) + errs, ok := unwrappedErr.(errutil.NonNilMultiError) if !ok { errs = []error{unwrappedErr} } diff --git a/pkg/receive/handler_test.go b/pkg/receive/handler_test.go index 4a06fdd286..70637d49fd 100644 --- a/pkg/receive/handler_test.go +++ b/pkg/receive/handler_test.go @@ -44,7 +44,7 @@ func TestDetermineWriteErrorCause(t *testing.T) { }, { name: "nil multierror", - err: errutil.MultiError([]error{}), + err: errutil.NonNilMultiError([]error{}), }, { name: "matching simple", @@ -54,7 +54,7 @@ func TestDetermineWriteErrorCause(t *testing.T) { }, { name: "non-matching multierror", - err: errutil.MultiError([]error{ + err: errutil.NonNilMultiError([]error{ errors.New("foo"), errors.New("bar"), }), @@ -62,7 +62,7 @@ func TestDetermineWriteErrorCause(t *testing.T) { }, { name: "nested non-matching multierror", - err: errors.Wrap(errutil.MultiError([]error{ + err: errors.Wrap(errutil.NonNilMultiError([]error{ errors.New("foo"), errors.New("bar"), }), "baz"), @@ -71,9 +71,9 @@ func TestDetermineWriteErrorCause(t *testing.T) { }, { name: "deep nested non-matching multierror", - err: errors.Wrap(errutil.MultiError([]error{ + err: errors.Wrap(errutil.NonNilMultiError([]error{ errors.New("foo"), - errutil.MultiError([]error{ + errutil.NonNilMultiError([]error{ errors.New("bar"), errors.New("qux"), }), @@ -83,7 +83,7 @@ func TestDetermineWriteErrorCause(t *testing.T) { }, { name: "matching multierror", - err: errutil.MultiError([]error{ + err: errutil.NonNilMultiError([]error{ storage.ErrOutOfOrderSample, errors.New("foo"), errors.New("bar"), @@ -93,7 +93,7 @@ func TestDetermineWriteErrorCause(t *testing.T) { }, { name: "matching but below threshold multierror", - err: errutil.MultiError([]error{ + err: errutil.NonNilMultiError([]error{ storage.ErrOutOfOrderSample, errors.New("foo"), errors.New("bar"), @@ -103,7 +103,7 @@ func TestDetermineWriteErrorCause(t *testing.T) { }, { name: "matching multierror many", - err: errutil.MultiError([]error{ + err: errutil.NonNilMultiError([]error{ storage.ErrOutOfOrderSample, errConflict, status.Error(codes.AlreadyExists, "conflict"), @@ -115,7 +115,7 @@ func TestDetermineWriteErrorCause(t *testing.T) { }, { name: "matching multierror many, one above threshold", - err: errutil.MultiError([]error{ + err: errutil.NonNilMultiError([]error{ storage.ErrOutOfOrderSample, errConflict, tsdb.ErrNotReady, @@ -128,7 +128,7 @@ func TestDetermineWriteErrorCause(t *testing.T) { }, { name: "matching multierror many, both above threshold, conflict have precedence", - err: errutil.MultiError([]error{ + err: errutil.NonNilMultiError([]error{ storage.ErrOutOfOrderSample, errConflict, tsdb.ErrNotReady, @@ -142,7 +142,7 @@ func TestDetermineWriteErrorCause(t *testing.T) { }, { name: "nested matching multierror", - err: errors.Wrap(errors.Wrap(errutil.MultiError([]error{ + err: errors.Wrap(errors.Wrap(errutil.NonNilMultiError([]error{ storage.ErrOutOfOrderSample, errors.New("foo"), errors.New("bar"), @@ -152,8 +152,8 @@ func TestDetermineWriteErrorCause(t *testing.T) { }, { name: "deep nested matching multierror", - err: errors.Wrap(errutil.MultiError([]error{ - errutil.MultiError([]error{ + err: errors.Wrap(errutil.NonNilMultiError([]error{ + errutil.NonNilMultiError([]error{ errors.New("qux"), status.Error(codes.AlreadyExists, "conflict"), status.Error(codes.AlreadyExists, "conflict"),