From 9b0cfc82bb41c995a3b6c3e3fae85a7cd3d589a6 Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Wed, 3 Jul 2024 18:44:18 +0200 Subject: [PATCH] Add Backoff.ErrCause() instead Signed-off-by: Marco Pracucci --- CHANGELOG.md | 2 +- backoff/backoff.go | 13 ++++++++++--- backoff/backoff_test.go | 26 ++++++++++++++++---------- 3 files changed, 27 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8bdf898ce..75c0b2824 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,7 +76,6 @@ * [CHANGE] Removed unused `time.Duration` parameter from `ShouldLog()` function in `middleware.OptionalLogging` interface. #513 * [CHANGE] Changed `ShouldLog()` function signature in `middleware.OptionalLogging` interface to `ShouldLog(context.Context) (bool, string)`: the returned `string` contains an optional reason. When reason is valued, `GRPCServerLog` adds `()` suffix to the error. #514 * [CHANGE] Cache: Remove superfluous `cache.RemoteCacheClient` interface and unify all caches using the `cache.Cache` interface. #520 -* [CHANGE] Backoff: `Backoff.Err()` now returns the context cancellation cause when provided to the context passed to `Backoff`. #538 * [FEATURE] Cache: Add support for configuring a Redis cache backend. #268 #271 #276 * [FEATURE] Add support for waiting on the rate limiter using the new `WaitN` method. #279 * [FEATURE] Add `log.BufferedLogger` type. #338 @@ -214,6 +213,7 @@ * [ENHANCEMENT] SpanProfiler: do less work on unsampled traces. #528 * [ENHANCEMENT] Log Middleware: if the trace is not sampled, log its ID as `trace_id_unsampled` instead of `trace_id`. #529 * [EHNANCEMENT] httpgrpc: httpgrpc Server can now use error message from special HTTP header when converting HTTP response to an error. This is useful when HTTP response body contains binary data that doesn't form valid utf-8 string, otherwise grpc would fail to marshal returned error. #531 +* [CHANGE] Backoff: added `Backoff.ErrCause()` which is like `Backoff.Err()` but returns the context cause if backoff is terminated because the context has been canceled. #538 * [BUGFIX] spanlogger: Support multiple tenant IDs. #59 * [BUGFIX] Memberlist: fixed corrupted packets when sending compound messages with more than 255 messages or messages bigger than 64KB. #85 * [BUGFIX] Ring: `ring_member_ownership_percent` and `ring_tokens_owned` metrics are not updated on scale down. #109 diff --git a/backoff/backoff.go b/backoff/backoff.go index 5468929bc..419af80e1 100644 --- a/backoff/backoff.go +++ b/backoff/backoff.go @@ -55,11 +55,9 @@ func (b *Backoff) Ongoing() bool { } // Err returns the reason for terminating the backoff, or nil if it didn't terminate. -// If backoff is terminated because the context has been canceled, then this function -// returns the context cancellation cause. func (b *Backoff) Err() error { if b.ctx.Err() != nil { - return context.Cause(b.ctx) + return b.ctx.Err() } if b.cfg.MaxRetries != 0 && b.numRetries >= b.cfg.MaxRetries { return fmt.Errorf("terminated after %d retries", b.numRetries) @@ -67,6 +65,15 @@ func (b *Backoff) Err() error { return nil } +// ErrCause is like Err() but returns the context cause if backoff is terminated because the +// context has been canceled. +func (b *Backoff) ErrCause() error { + if b.ctx.Err() != nil { + return context.Cause(b.ctx) + } + return b.Err() +} + // NumRetries returns the number of retries so far func (b *Backoff) NumRetries() int { return b.numRetries diff --git a/backoff/backoff_test.go b/backoff/backoff_test.go index 09379908d..b9caaf2fc 100644 --- a/backoff/backoff_test.go +++ b/backoff/backoff_test.go @@ -109,44 +109,49 @@ func TestBackoff_Err(t *testing.T) { cause := errors.New("my cause") tests := map[string]struct { - ctx func(*testing.T) context.Context - expectedErr error + ctx func(*testing.T) context.Context + expectedErr error + expectedErrCause error }{ - "should return context.DeadlineExceeded when context deadline exceeded without cause": { + "context deadline exceeded without cause": { ctx: func(t *testing.T) context.Context { ctx, cancel := context.WithDeadline(context.Background(), time.Now()) t.Cleanup(cancel) return ctx }, - expectedErr: context.DeadlineExceeded, + expectedErr: context.DeadlineExceeded, + expectedErrCause: context.DeadlineExceeded, }, - "should return cause when context deadline exceeded with cause": { + "context deadline exceeded with cause": { ctx: func(t *testing.T) context.Context { ctx, cancel := context.WithDeadlineCause(context.Background(), time.Now(), cause) t.Cleanup(cancel) return ctx }, - expectedErr: cause, + expectedErr: context.DeadlineExceeded, + expectedErrCause: cause, }, - "should return context.Canceled when context is canceled without cause": { + "context is canceled without cause": { ctx: func(_ *testing.T) context.Context { ctx, cancel := context.WithCancel(context.Background()) cancel() return ctx }, - expectedErr: context.Canceled, + expectedErr: context.Canceled, + expectedErrCause: context.Canceled, }, - "should return cause when context is canceled with cause": { + "context is canceled with cause": { ctx: func(_ *testing.T) context.Context { ctx, cancel := context.WithCancelCause(context.Background()) cancel(cause) return ctx }, - expectedErr: cause, + expectedErr: context.Canceled, + expectedErrCause: cause, }, } @@ -160,6 +165,7 @@ func TestBackoff_Err(t *testing.T) { }, time.Second, 10*time.Millisecond) require.Equal(t, testData.expectedErr, b.Err()) + require.Equal(t, testData.expectedErrCause, b.ErrCause()) }) } }