CircuitBreakerAspect loggging side-effect bug

[narusas]

When some circuit breaker is down, If I enable CircuitBreakerAspect debug log,
CircuitBreakerAspect: line 108 call circuiteBreaker.getStatus() and getServiceStatus(). getServiceStatus() call allowRequest() and allowRequest() call canAttempt() and it change isAttempLive to true and circuitbreaker status broken.

Test Code

@Test
public void circuitBreakerRecovery() throws InterruptedException {
    for (int i = 0; i <= 3; i++) {
        try {
            bean.doSomething2();
            fail("raise exception");
        }
        catch (IllegalArgumentException ex) {
            // I want it 
        }
    }

    try {
        bean.doSomething2();
    }
    catch (CircuitBreakerException ex) {
        log.info("Now circuit opened");
    }

    Thread.sleep(2000L);
    bean.setError(false);
    log.info("Change  no more error in service");
    bean.doSomething2();
    assertTrue("No execption here", true);
}

Fixture

@CircuitBreaker(name = "cb2", limit = 3, resetMillis = 100L, windowMillis = 100L)
public void doSomething2() {
    if (error) {
        throw new IllegalArgumentException();
    }
}  

[joercampbell]

Thanks for posting -

Can you clarify for me - Are you 'expecting' there to be an error at the last assert or not? Is it a log message you are expecting to occur? If you set the error on the bean to false (bean.setError(false)), it won't be behave the same way as if you had bean.reset() the breaker.

[narusas]

Last assertion is "NOT" expect throw exception. I want circuitbreaker has turn back normal status.

When I enable debug log, jrugged try to log service status. But making log string, it call canAttempt() and canAttemp change circuitbreaker's isAttempLive status.

Once isAttempLive has chaged, it never rollback status after reset time.

I mean getServiceStatus() change status.(isAttempLive)

I think getServiceStatus() separated to query method and command method.

[joercampbell]

The head version of jrugged actually doesn't mutate isAttemptLive on that call chain anymore. Can you pull Head and try your test again just to make sure we have actually fixed the issue you are describing here?

[joercampbell]

Fixed in the release I am about to do.