From 0ced27ed5abf502f2804ccdf6fe0cc75d93af271 Mon Sep 17 00:00:00 2001 From: Leo Antunes Date: Fri, 29 Dec 2023 16:22:31 +0100 Subject: [PATCH] fix!: use errors.Is for context.Cancelled check BREAKING: also rename IgnoreContextCancellation to IgnoreContextCancelled --- README.md | 4 ++-- hoglet.go | 10 +++++----- hoglet_test.go | 2 +- limiter_test.go | 2 +- options.go | 7 ++++--- 5 files changed, 13 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index de9132f..92671ba 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ h, err := hoglet.NewCircuit( return Foo{}, fmt.Errorf("bar is not 42") }, hoglet.NewSlidingWindowBreaker(10, 0.1), - hoglet.WithFailureCondition(hoglet.IgnoreContextCancelation), + hoglet.WithFailureCondition(hoglet.IgnoreContextCanceled), ) /* if err != nil ... */ @@ -38,7 +38,7 @@ these observations according to their own logic, optionally opening the circuit. An open circuit does not allow any calls to go through, and will return an error immediately. -If the wrapped function blocks, `Circuit.Call` will block as well, but any context cancelations or expirations will +If the wrapped function blocks, `Circuit.Call` will block as well, but any context cancellations or expirations will count towards the failure rate, allowing the circuit to respond timely to failures, while still having well-defined and non-racy behavior around the failed function. diff --git a/hoglet.go b/hoglet.go index 7f58740..8fcbeb7 100644 --- a/hoglet.go +++ b/hoglet.go @@ -195,7 +195,7 @@ func (s stateObserver[IN, OUT]) Observe(failure bool) { // ensures the circuit opens quickly, even if the wrapped function blocks. // // By default, all errors are considered failures (including [context.Canceled]), but this can be customized via -// [WithFailureCondition] and [IgnoreContextCancelation]. +// [WithFailureCondition] and [IgnoreContextCanceled]. // // Panics are observed as failures, but are not recovered (i.e.: they are "repanicked" instead). func (c *Circuit[IN, OUT]) Call(ctx context.Context, in IN) (out OUT, err error) { @@ -224,7 +224,7 @@ func (c *Circuit[IN, OUT]) Call(ctx context.Context, in IN) (out OUT, err error) obs = dedupObservableCall(obs) obsCtx, cancel := context.WithCancelCause(ctx) - defer cancel(internalCancellation) + defer cancel(errWrappedFunctionDone) // TODO: we could skip this if we could ensure the original context has neither cancellation nor deadline go c.observeCtx(obs, obsCtx) @@ -241,8 +241,8 @@ func (c *Circuit[IN, OUT]) Call(ctx context.Context, in IN) (out OUT, err error) return c.f(ctx, in) } -// internalCancellation is used to distinguish between internal and external (to the lib) context cancellations. -var internalCancellation = errors.New("internal cancellation") +// errWrappedFunctionDone is used to distinguish between internal and external (to the lib) context cancellations. +var errWrappedFunctionDone = errors.New("wrapped function done") // observeCtx observes the given context for cancellation and records it as a failure. // It assumes [Observer] is idempotent and deduplicates calls itself. @@ -252,7 +252,7 @@ func (c *Circuit[IN, OUT]) observeCtx(obs Observer, ctx context.Context) { <-ctx.Done() err := ctx.Err() - if context.Cause(ctx) == internalCancellation { + if context.Cause(ctx) == errWrappedFunctionDone { err = nil // ignore internal cancellations; the wrapped function returned already } obs.Observe(c.options.isFailure(err)) diff --git a/hoglet_test.go b/hoglet_test.go index d8f02dd..1f2f857 100644 --- a/hoglet_test.go +++ b/hoglet_test.go @@ -99,7 +99,7 @@ func TestCircuit_ignored_context_cancellation_still_returned(t *testing.T) { return "expected", ctx.Err() }, nil, - WithFailureCondition(IgnoreContextCancelation)) + WithFailureCondition(IgnoreContextCanceled)) require.NoError(t, err) ctx, cancel := context.WithCancel(context.Background()) diff --git a/limiter_test.go b/limiter_test.go index 46406d4..f0cfb62 100644 --- a/limiter_test.go +++ b/limiter_test.go @@ -59,7 +59,7 @@ func Test_ConcurrencyLimiter(t *testing.T) { wantErr: hoglet.ErrWaitingForSlot, }, { - name: "cancelation releases with error", + name: "cancellation releases with error", args: args{limit: 1, block: true}, calls: 1, cancel: true, diff --git a/options.go b/options.go index 84cc2e2..6784c69 100644 --- a/options.go +++ b/options.go @@ -2,6 +2,7 @@ package hoglet import ( "context" + "errors" "fmt" "time" ) @@ -38,9 +39,9 @@ func WithFailureCondition(condition func(error) bool) Option { }) } -// IgnoreContextCancelation is a helper function for [WithFailureCondition] that ignores [context.Canceled] errors. -func IgnoreContextCancelation(err error) bool { - return err != nil && err != context.Canceled +// IgnoreContextCanceled is a helper function for [WithFailureCondition] that ignores [context.Canceled] errors. +func IgnoreContextCanceled(err error) bool { + return err != nil && !errors.Is(err, context.Canceled) } // WithMiddleware allows wrapping the [Breaker] via a [BreakerMiddleware].