fix: guard isFailure with nil check to simplify failure conditions

breaker_test.go

@@ -167,9 +167,7 @@ func TestBreaker_Observe_State(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
-		tt := tt
 		for bName, b := range tt.breakers {
-			b := b
 			t.Run(bName+": "+tt.name, func(t *testing.T) {
 				t.Parallel()
 

hoglet.go

@@ -236,7 +236,7 @@ func Wrap[IN, OUT any](c *Circuit, f WrappableFunc[IN, OUT]) WrappableFunc[IN, O
 				obs.Observe(true)
 				panic(err) // let the caller deal with panics
 			}
-			obs.Observe(c.options.isFailure(err))
+			obs.Observe(err != nil && c.options.isFailure(err))
 		}()
 
 		return f(ctx, in)
@@ -257,7 +257,7 @@ func (c *Circuit) observeCtx(obs Observer, ctx context.Context) {
 	if context.Cause(ctx) == errWrappedFunctionDone {
 		err = nil // ignore internal cancellations; the wrapped function returned already
 	}
-	obs.Observe(c.options.isFailure(err))
+	obs.Observe(err != nil && c.options.isFailure(err))
 }
 
 // State represents the state of a circuit.

hoglet_test.go

@@ -10,7 +10,7 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-var sentinel = errors.New("sentinel error")
+var errSentinel = errors.New("sentinel error")
 
 type noopIn int
 
@@ -26,7 +26,7 @@ func noop(ctx context.Context, in noopIn) (struct{}, error) {
 	case noopInSuccess:
 		return struct{}{}, nil
 	case noopInFailure:
-		return struct{}{}, sentinel
+		return struct{}{}, errSentinel
 	default: // noopInPanic
 		panic("boom")
 	}
@@ -80,9 +80,9 @@ func TestBreaker_nil_breaker_does_not_open(t *testing.T) {
 	b, err := NewCircuit(nil)
 	require.NoError(t, err)
 	_, err = Wrap(b, noop)(t.Context(), noopInFailure)
-	assert.Equal(t, sentinel, err)
+	assert.Equal(t, errSentinel, err)
 	_, err = Wrap(b, noop)(t.Context(), noopInFailure)
-	assert.Equal(t, sentinel, err)
+	assert.Equal(t, errSentinel, err)
 }
 
 func TestBreaker_ctx_parameter_not_cancelled(t *testing.T) {
@@ -152,7 +152,7 @@ func TestHoglet_Do(t *testing.T) {
 			name: "error opens",
 			calls: []calls{
 				{arg: noopInSuccess, wantErr: nil},
-				{arg: noopInFailure, wantErr: sentinel},
+				{arg: noopInFailure, wantErr: errSentinel},
 				{arg: noopInSuccess, wantErr: ErrCircuitOpen},
 			},
 		},
@@ -168,7 +168,7 @@ func TestHoglet_Do(t *testing.T) {
 			name: "success on half-open closes",
 			calls: []calls{
 				{arg: noopInSuccess, wantErr: nil},
-				{arg: noopInFailure, wantErr: sentinel},
+				{arg: noopInFailure, wantErr: errSentinel},
 				{arg: noopInSuccess, wantErr: nil, halfOpen: true},
 				{arg: noopInSuccess, wantErr: nil},
 			},
@@ -177,8 +177,8 @@ func TestHoglet_Do(t *testing.T) {
 			name: "failure on half-open keeps open",
 			calls: []calls{
 				{arg: noopInSuccess, wantErr: nil},
-				{arg: noopInFailure, wantErr: sentinel},
-				{arg: noopInFailure, wantErr: sentinel, halfOpen: true},
+				{arg: noopInFailure, wantErr: errSentinel},
+				{arg: noopInFailure, wantErr: errSentinel, halfOpen: true},
 				{arg: noopInSuccess, wantErr: ErrCircuitOpen},
 			},
 		},
@@ -207,11 +207,27 @@ func TestHoglet_Do(t *testing.T) {
 	}
 }
 
+func TestCircuit_failure_condition_never_called_with_nil_error(t *testing.T) {
+	conditionCalled := false
+	condition := func(err error) bool {
+		conditionCalled = true
+		require.NotNil(t, err, "failure condition must not be called with nil error")
+		return true
+	}
+
+	b, err := NewCircuit(nil, WithFailureCondition(condition))
+	require.NoError(t, err)
+
+	_, err = Wrap(b, noop)(t.Context(), noopInSuccess)
+	require.NoError(t, err)
+	assert.False(t, conditionCalled, "failure condition should not have been called for a successful call")
+}
+
 // maybeAssertPanic is a test-table helper to assert that a function panics or not, depending on the value of wantPanic.
 func maybeAssertPanic(t *testing.T, f func(), wantPanic any) {
 	wrapped := assert.NotPanics
 	if wantPanic != nil {
-		wrapped = func(t assert.TestingT, f assert.PanicTestFunc, msgAndArgs ...interface{}) bool {
+		wrapped = func(t assert.TestingT, f assert.PanicTestFunc, msgAndArgs ...any) bool {
 			return assert.PanicsWithValue(t, wantPanic, f, msgAndArgs...)
 		}
 	}

options.go

@@ -29,9 +29,9 @@ func WithHalfOpenDelay(delay time.Duration) Option {
 // WithFailureCondition allows specifying a filter function that determines whether an error should open the breaker.
 // If the provided function returns true, the error is considered a failure and the breaker may open (depending on the
 // breaker logic).
-// The default filter considers all non-nil errors as failures (err != nil).
+// Nil errors are always considered successes. The provided function is only called in the non-nil error case.
 //
-// This does not modify the error returned by [Circuit.Call]. It only affects the circuit itself.
+// This does not modify the error returned by the wrapped function call. It only affects the circuit itself.
 func WithFailureCondition(condition func(error) bool) Option {
 	return optionFunc(func(o *options) error {
 		o.isFailure = condition
@@ -41,7 +41,7 @@ func WithFailureCondition(condition func(error) bool) Option {
 
 // 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)
+	return !errors.Is(err, context.Canceled)
 }
 
 // WithBreakerMiddleware allows wrapping the [Breaker] via a [BreakerMiddleware].