From 942459cb28bbd706613b501ad0a9a24b9d019ae9 Mon Sep 17 00:00:00 2001 From: Leo Antunes Date: Fri, 9 Oct 2026 11:57:14 +0200 Subject: [PATCH 1/2] fix: only let a half-open call close an open circuit Any call reporting a success the breaker deemed healthy enough closed the circuit, including calls admitted before it opened and still in flight. Those say nothing about whether the upstream recovered, so they cut the half-open delay short and let the full load back in without a probe. The circuit now only applies a close for calls admitted half-open. Any call can still open it. Co-Authored-By: Claude Opus 5.5 --- hoglet.go | 6 +++++- hoglet_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/hoglet.go b/hoglet.go index aec422b..be341fd 100644 --- a/hoglet.go +++ b/hoglet.go @@ -234,7 +234,11 @@ func (s stateObserver) Observe(failure bool) { case stateChangeOpen: s.circuit.open() case stateChangeClose: - s.circuit.close() + // Only a call admitted half-open may close the circuit. A call still in flight from before the circuit opened + // says nothing about whether it has recovered, and would cut the half-open delay short. + if s.state == StateHalfOpen { + s.circuit.close() + } } } diff --git a/hoglet_test.go b/hoglet_test.go index 6f4fb8b..fbc2b2a 100644 --- a/hoglet_test.go +++ b/hoglet_test.go @@ -230,6 +230,31 @@ func TestHoglet_Do(t *testing.T) { } } +func TestCircuit_call_admitted_before_opening_does_not_close_it(t *testing.T) { + c, err := NewCircuit(&mockBreaker{}, WithHalfOpenDelay(time.Minute)) + require.NoError(t, err) + + started, release := make(chan struct{}), make(chan struct{}) + done := make(chan error) + go func() { + _, err := Wrap(c, func(ctx context.Context, in noopIn) (struct{}, error) { + close(started) + <-release + return noop(ctx, in) + })(context.Background(), noopInSuccess) + done <- err + }() + <-started // admitted while closed + + _, err = Wrap(c, noop)(context.Background(), noopInFailure) + require.ErrorIs(t, err, errSentinel) + require.Equal(t, StateOpen, c.State()) + + close(release) + require.NoError(t, <-done) + assert.Equal(t, StateOpen, c.State(), "only a half-open call may close the circuit") +} + func TestCircuit_ignored_context_error_does_not_mask_wrapped_function_result(t *testing.T) { synctest.Test(t, func(t *testing.T) { c, err := NewCircuit(&mockBreaker{}, WithHalfOpenDelay(time.Minute), WithFailureCondition(IgnoreContextCanceled)) From 0339735c4f5b203143ddd02c88c716f73cd5da44 Mon Sep 17 00:00:00 2001 From: Leo Antunes Date: Fri, 9 Oct 2026 12:54:26 +0200 Subject: [PATCH 2/2] fix: only let a successful half-open call close the circuit A call admitted before the circuit opened could no longer close it, but its success still lowered the breaker's failure rate. A failing half-open call could then come out at or below the threshold, and the circuit closed on it, since it was admitted half-open. Close only on a successful half-open call. The failure rate the breaker keeps while the circuit is open then has no effect, and a successful half-open call resets both breakers, so stale successes leave no trace. The regression test now uses both real breakers, since the mock breaker never reports a close for a failure. Co-Authored-By: Claude Opus 5.5 --- hoglet.go | 7 ++--- hoglet_test.go | 73 ++++++++++++++++++++++++++++++++++---------------- 2 files changed, 54 insertions(+), 26 deletions(-) diff --git a/hoglet.go b/hoglet.go index be341fd..4998db3 100644 --- a/hoglet.go +++ b/hoglet.go @@ -234,9 +234,10 @@ func (s stateObserver) Observe(failure bool) { case stateChangeOpen: s.circuit.open() case stateChangeClose: - // Only a call admitted half-open may close the circuit. A call still in flight from before the circuit opened - // says nothing about whether it has recovered, and would cut the half-open delay short. - if s.state == StateHalfOpen { + // Only a successful half-open call may close the circuit. A call still in flight from before the circuit opened + // says nothing about whether it has recovered, and its success may still lower the failure rate enough for a + // failing half-open call to come out below the threshold. + if s.state == StateHalfOpen && !failure { s.circuit.close() } } diff --git a/hoglet_test.go b/hoglet_test.go index fbc2b2a..72b9fe5 100644 --- a/hoglet_test.go +++ b/hoglet_test.go @@ -230,29 +230,56 @@ func TestHoglet_Do(t *testing.T) { } } -func TestCircuit_call_admitted_before_opening_does_not_close_it(t *testing.T) { - c, err := NewCircuit(&mockBreaker{}, WithHalfOpenDelay(time.Minute)) - require.NoError(t, err) - - started, release := make(chan struct{}), make(chan struct{}) - done := make(chan error) - go func() { - _, err := Wrap(c, func(ctx context.Context, in noopIn) (struct{}, error) { - close(started) - <-release - return noop(ctx, in) - })(context.Background(), noopInSuccess) - done <- err - }() - <-started // admitted while closed - - _, err = Wrap(c, noop)(context.Background(), noopInFailure) - require.ErrorIs(t, err, errSentinel) - require.Equal(t, StateOpen, c.State()) - - close(release) - require.NoError(t, <-done) - assert.Equal(t, StateOpen, c.State(), "only a half-open call may close the circuit") +func TestCircuit_only_successful_half_open_call_closes(t *testing.T) { + tests := []struct { + name string + breaker func() Breaker + }{ + {name: "ewma", breaker: func() Breaker { return NewEWMABreaker(10, 0.9) }}, + {name: "slidingwindow", breaker: func() Breaker { return NewSlidingWindowBreaker(time.Minute, 0.5) }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + c, err := NewCircuit(tt.breaker(), WithHalfOpenDelay(time.Second)) + require.NoError(t, err) + + // two calls admitted while closed, which only succeed after the circuit opened + release := make(chan struct{}) + done := make(chan error) + for range 2 { + go func() { + _, err := Wrap(c, func(ctx context.Context, in noopIn) (struct{}, error) { + <-release + return noop(ctx, in) + })(context.Background(), noopInSuccess) + done <- err + }() + } + synctest.Wait() + + _, err = Wrap(c, noop)(context.Background(), noopInFailure) + require.ErrorIs(t, err, errSentinel) + require.Equal(t, StateOpen, c.State()) + + close(release) + require.NoError(t, <-done) + require.NoError(t, <-done) + assert.Equal(t, StateOpen, c.State(), "calls admitted before the circuit opened must not close it") + + // their successes lowered the failure rate enough for a failing half-open call to stay below the threshold + time.Sleep(c.halfOpenDelay) + _, err = Wrap(c, noop)(context.Background(), noopInFailure) + require.ErrorIs(t, err, errSentinel) + assert.Equal(t, StateOpen, c.State(), "a failed half-open call must not close the circuit") + + time.Sleep(c.halfOpenDelay) + _, err = Wrap(c, noop)(context.Background(), noopInSuccess) + require.NoError(t, err) + assert.Equal(t, StateClosed, c.State(), "a successful half-open call must close the circuit") + }) + }) + } } func TestCircuit_ignored_context_error_does_not_mask_wrapped_function_result(t *testing.T) {