Issue ReactiveX#544: Fixes bug that CircuitBreaker stays in half open…

… when exceptions are ignored. (ReactiveX#545)
jihoonson · Jul 15, 2019 · 5dbf2d1 · 5dbf2d1
1 parent 1a87d08
commit 5dbf2d1
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 0 deletions.
diff --git a/.../main/java/io/github/resilience4j/circuitbreaker/internal/CircuitBreakerStateMachine.java b/.../main/java/io/github/resilience4j/circuitbreaker/internal/CircuitBreakerStateMachine.java
@@ -167,6 +167,7 @@ private void handleThrowable(long durationInNanos, Predicate<Throwable> recordFa
             publishCircuitErrorEvent(name, durationInNanos, throwable);
             stateReference.get().onError(throwable);
         } else {
+            releasePermission();
             publishCircuitIgnoredErrorEvent(name, durationInNanos, throwable);
         }
     }

diff --git a/...t/java/io/github/resilience4j/circuitbreaker/internal/CircuitBreakerStateMachineTest.java b/...t/java/io/github/resilience4j/circuitbreaker/internal/CircuitBreakerStateMachineTest.java
@@ -314,6 +314,38 @@ public void shouldForceOpenCircuitBreaker() {
         assertCircuitBreakerMetricsEqualTo(-1f, 0, 0, 4, 0, 2L);
     }
 
+    @Test
+    public void shouldReleasePermissionWhenExceptionIgnored() {
+        circuitBreaker.transitionToOpenState();
+        circuitBreaker.transitionToHalfOpenState();
+        assertThat(circuitBreaker.getState()).isEqualTo(CircuitBreaker.State.HALF_OPEN);
+
+        assertThat(circuitBreaker.tryAcquirePermission()).isEqualTo(true);
+        circuitBreaker.onSuccess(0); // Should create a CircuitBreakerOnSuccessEvent
+        assertThat(circuitBreaker.getState()).isEqualTo(CircuitBreaker.State.HALF_OPEN);
+        assertCircuitBreakerMetricsEqualTo(-1f, 1, 1, 4, 0, 0L);
+
+        assertThat(circuitBreaker.tryAcquirePermission()).isEqualTo(true);
+        circuitBreaker.onSuccess(0); // Should create a CircuitBreakerOnSuccessEvent
+        assertThat(circuitBreaker.getState()).isEqualTo(CircuitBreaker.State.HALF_OPEN);
+        assertCircuitBreakerMetricsEqualTo(-1f, 2, 2, 4, 0, 0L);
+
+        assertThat(circuitBreaker.tryAcquirePermission()).isEqualTo(true);
+        circuitBreaker.onSuccess(0); // Should create a CircuitBreakerOnSuccessEvent
+        assertThat(circuitBreaker.getState()).isEqualTo(CircuitBreaker.State.HALF_OPEN);
+        assertCircuitBreakerMetricsEqualTo(-1f, 3, 3, 4, 0, 0L);
+
+        assertThat(circuitBreaker.tryAcquirePermission()).isEqualTo(true);
+        circuitBreaker.onError(0, new NumberFormatException()); // Should create a CircuitBreakerOnErrorEvent
+        assertThat(circuitBreaker.getState()).isEqualTo(CircuitBreaker.State.HALF_OPEN);
+        assertCircuitBreakerMetricsEqualTo(-1f, 3, 3, 4, 0, 0L);
+
+        assertThat(circuitBreaker.tryAcquirePermission()).isEqualTo(true);
+        circuitBreaker.onError(0, new RuntimeException()); // Should create a CircuitBreakerOnErrorEvent
+        assertThat(circuitBreaker.getState()).isEqualTo(CircuitBreaker.State.CLOSED);
+        assertCircuitBreakerMetricsEqualTo(-1f, 3, 4, 5, 1, 0L);
+    }
+
     private void assertCircuitBreakerMetricsEqualTo(Float expectedFailureRate, Integer expectedSuccessCalls, Integer expectedBufferedCalls, Integer expectedMaxBufferedCalls, Integer expectedFailedCalls, Long expectedNotPermittedCalls) {
         final CircuitBreaker.Metrics metrics = circuitBreaker.getMetrics();
         assertThat(metrics.getFailureRate()).isEqualTo(expectedFailureRate);