fail_max value is not honored

[mardiros]

Hi @arlyon

I am currently adding circuit breaker in the rest api client I am working on,

From my followin test, the fail_max value sounds wrong to me.

https://github.com/mardiros/blacksmith/blob/master/tests/unittests/test_middleware.py#L206-220

When I say fail_max=2, I am expecting to do 2 called before the circuit breaker is open, not (fail_max=1)

Could you confirm ?

[arlyon]

Hi! Apologies for the late reply, I've been away recently. Hope you're doing well.

When fail_max is two, the underlying method will be allowed to raise an exception at most two times. This exception will be raised as usual. When the number of failures is equal to or greater than fail_max, additional attempts will be prevented and a circuit breaker error will be raised instead until a timeout passes. Once the timeout passes, the breaker will attempt to call the underlying method again. A failure here will open the breaker again, while a success will reset the number of failures to 0.

This functionality can be verified from the tests: https://github.com/arlyon/aiobreaker/blob/master/test/test_circuit.py

[mardiros]

Hi, thanks for the reply.

From what I see here:

https://github.com/arlyon/aiobreaker/blob/master/test/test_circuit.py#L81

There is a circuit breaker initialized with fail_max=3 and the DummyException is raised only 2 times which is not what expected,
from what I understand of your comment. ( When the number of failures is equal to or greater than fail_max,.. )
2 is not equal or greater than 3

[arlyon]

Ah, yes, I agree the wording is a bit complicated here.

aiobreaker/aiobreaker/state.py

Line 176 in 9b8ac0e

if self._breaker._state_storage.counter >= self._breaker.fail_max:

If you look here, the closed state listens for failed calls and raises a circuit breaker error. So, if max_fail is 3, the function is in fact called 3 times, however rather than raising the underlying error on the 3rd failure, a breaker exception is raised. I agree that this is potentially confusing. To make this behaviour clear in the tests I will adjust the test to assert the number of calls on func_exception