CircuitBreaker: unhandled exceptions can count as the single attempt permitted in half-open state

[brunoabreu]

I'm using a CircuitBreaker to wrap some HTTP calls.
My HTTP client has a timeout and sometimes it throws a TaskCanceledException.

I'm not handling that exception on my CircuitBreaker and it works fine, not transitioning from closed to open state when a timeout happens. But when the circuit is half-open, that exception prevents it to be closed.

It seems like any exception thrown during half-open state, handled or not, opens the circuit.

Is that an expected behavior?

[reisenberger]

@brunoabreu . That's a great question. It should not be the case. The circuit-breaker engine explicitly rethrows unhandled execptions (TaskCanceledException in your case), without them affecting circuit state or metrics (unhandled exceptions do not cause a call in to OnActionFailure(...)).

I was able to construct a quick test, showing that the unhandled exception leaves the circuit in half-open state:

[Fact]
public void Should_leave_circuit_halfopen_if_the_next_call_raises_an_unhandled_exception()
{
    var time = 1.January(2000);
    SystemClock.UtcNow = () => time;

    var durationOfBreak = TimeSpan.FromMinutes(1);

    CircuitBreakerPolicy breaker = Policy
        .Handle<DivideByZeroException>()
        .CircuitBreaker(2, durationOfBreak);

    breaker.Invoking(x => x.RaiseException<DivideByZeroException>())
        .ShouldThrow<DivideByZeroException>();
    breaker.CircuitState.Should().Be(CircuitState.Closed);

    breaker.Invoking(x => x.RaiseException<DivideByZeroException>())
        .ShouldThrow<DivideByZeroException>();
    breaker.CircuitState.Should().Be(CircuitState.Open);

    // 2 exception raised, circuit is now open
    breaker.Invoking(x => x.RaiseException<DivideByZeroException>())
        .ShouldThrow<BrokenCircuitException>();
    breaker.CircuitState.Should().Be(CircuitState.Open);


    // duration has passed, circuit now half open
    SystemClock.UtcNow = () => time.Add(durationOfBreak);
    breaker.CircuitState.Should().Be(CircuitState.HalfOpen);

    // first call after duration raises an unhandled exception
    breaker.Invoking(x => x.RaiseException<TaskCanceledException>())
        .ShouldThrow<TaskCanceledException>();
    // unhandled exception should not affect state - breaker should still be halfopen
    breaker.CircuitState.Should().Be(CircuitState.HalfOpen);
}

Could what you are seeing be that the circuit-breaker only permits one execution attempt per breakDuration, in half-open state?

So: If, after the first TaskCanceledException in half-open state, your breakDuration has still not expired, then a second call (before breakDuration expires) will throw BrokenCircuitException, even though the circuit is still in half-open state - because only one trial call per breakDuration in half-open state is permitted.

Does that sound like it describes your scenario?

There are tests showing this here. Adding the following lines to the end of the test above, also demonstrate this:

    breaker.Invoking(x => x.Execute(() => {})).ShouldThrow<BrokenCircuitException>();
    breaker.CircuitState.Should().Be(CircuitState.HalfOpen);

The behaviour is by design, to prevent request stampedes during half-open state, and matches Hystrix's approach.

---

If that doesn't describe your scenario (or if qs about it), let me know, and we'll dig deeper.

[brunoabreu]

@reisenberger thank you for your explanation. I wasn't aware of this single trial call per breakDuration in half-open state. I think that is exactly what is happening here.

Although I understand this strategy, it seems a little odd because if I'm not handling some kind of exception and it happens, I don't expect it to be treated as a failure that prevents my circuit breaker from transitioning half-open -> closed.

If it is closed, that inertia makes sense. Unhandled exception should not interfere in circuit state. But when we apply that rule in half-open case, things become a little confusing.

[pvmraghunandan]

@reisenberger Quick question. I am facing similar issue. When the circuit is in open state and transistioned to half open and at the same time if we get two calls, are we saying that we allow only one call to pass through and other call will get Broken Circuit Exception? I am seeing similar behavior. If that's case, can we skip whole half-open? :)

[reisenberger]

@pvmraghunandan Quick answers:

When the circuit is in open state and transistioned to half open and at the same time if we get two calls, are we saying that we allow only one call to pass through and other call will get Broken Circuit Exception?

Yes, as documented in the circuit-breaker wiki. I've made the doco clearer to confirm that blocked executions in half-open also throw BrokenCircuitException.

If that's case, can we skip whole half-open? :)

No built-in Polly syntax to do that. But you could achieve it by configuring the onHalfOpen: delegate just to call breakerPolicy.Reset(); - that will make the circuit-breaker transition directly back to closed when it hits half-open. Of course, you don't have access to the breakerPolicy instance at the point of configuring the policy, so you'd have to do the same trick we discussed few days ago where you pass data in via Context - this time passing in the policy you are executing through via Context, so that onHalfOpen can get it from context, then call Reset() on it.

We may later introduce an injectible circuit-breaker controller you could custom-code to control circuit-breaker behaviour more finely.

[amiyaaloke]

onHalfOpen delegate doesn't have context. How to you propose to use context to pass in breakerPolicy to be reset in onHalfOpen?

[reisenberger]

@amiyaaloke You are correct: apologies for getting that wrong. In that case, the approach outlined above is not achievable with current Polly. I remember now also why onHalfOpen is the sole delegate hook in Polly not to take Context as an input parameter. A circuit may transition from open->halfOpen just when the circuit status is checked, which can happen outside the execution of a Policy, so no Context is available in that case. This invalidates the previously suggested approach as a way to achieve a Polly circuit-breaker which spends no time in a half-open state.

As part of #287 we intend to introduce an injectible ICircuitController concept; you could thus replace Polly's default circuit-controller with your own ICircuitController with variant behaviour. This would allow you to code custom behaviour for the half-open state; for instance a custom circuit-breaker which spent no time in half-open.

[amiyaaloke]

@reisenberger thanks for the explanation. As a workaround I added code to cache policy registry and onHalfOpen callback reset the circuit breakers that are on HalfOpen state to close the circuit. Let me know if you see any downside of this approach.

[reisenberger]

@amiyaaloke Sounds to me like that should work 👍

[amiyaaloke]

@reisenberger Yes. this seems to working well with lock in place to make it thread safe. Thanks for reply 👍

[reisenberger]

Closing due to no further comments on the original issue in a couple of years.