diff --git a/lib/semian/circuit_breaker.rb b/lib/semian/circuit_breaker.rb index f9ee5127d..d7aa40119 100644 --- a/lib/semian/circuit_breaker.rb +++ b/lib/semian/circuit_breaker.rb @@ -77,10 +77,14 @@ def mark_failed(error) end def mark_success - return unless half_open? - - @successes.increment - transition_to_close if success_threshold_reached? + if half_open? + @successes.increment + if success_threshold_reached? + transition_to_close + end + else + @errors.clear unless error_threshold_timeout_enabled + end end def reset diff --git a/test/circuit_breaker_test.rb b/test/circuit_breaker_test.rb index 3b3fd7c2f..4a1728542 100644 --- a/test/circuit_breaker_test.rb +++ b/test/circuit_breaker_test.rb @@ -707,4 +707,38 @@ def test_lumping_interval_cannot_be_greater_than_error_threshold_timeout ensure Semian.destroy(:lumping_validation_test) end + + def test_consecutive_errors_only_when_error_threshold_timeout_enabled_false + resource = Semian.register( + :consecutive_errors_test, + bulkhead: false, + exceptions: [SomeError], + error_threshold: 2, + error_timeout: 5, + success_threshold: 1, + error_threshold_timeout_enabled: false, + ) + + # First error + trigger_error!(resource) + + assert_circuit_closed(resource) + + # Many successful requests + 100.times do + resource.acquire { true } + end + + # Second error should NOT open the circuit because it's not consecutive + trigger_error!(resource) + + assert_circuit_closed(resource) + + # Third error should open the circuit (we now have 2 consecutive errors) + trigger_error!(resource) + + assert_circuit_opened(resource) + ensure + Semian.destroy(:consecutive_errors_test) + end end diff --git a/test/helpers/circuit_breaker_helper.rb b/test/helpers/circuit_breaker_helper.rb index c5db6f879..eb4e2914e 100644 --- a/test/helpers/circuit_breaker_helper.rb +++ b/test/helpers/circuit_breaker_helper.rb @@ -22,8 +22,21 @@ def trigger_error!(resource = @resource, error = SomeError) def assert_circuit_closed(resource = @resource) block_called = false + + circuit_breaker = resource.circuit_breaker + + old_errors = create_sliding_window_copy(circuit_breaker.instance_variable_get(:@errors)) + previously_open = circuit_breaker&.send(:open?) + previously_half_open = circuit_breaker&.send(:half_open?) + resource.acquire { block_called = true } + now_closed = !circuit_breaker&.send(:open?) + + unless previously_half_open || previously_open && now_closed || circuit_breaker.instance_variable_get(:@error_threshold_timeout_enabled).nil? || circuit_breaker.instance_variable_get(:@error_threshold_timeout_enabled) + circuit_breaker.instance_variable_set(:@errors, old_errors) + end + assert(block_called, "Expected the circuit to be closed, but it was open") end @@ -37,4 +50,21 @@ def assert_circuit_opened(resource = @resource) assert(open, "Expected the circuit to be open, but it was closed") end + + def create_sliding_window_copy(sliding_window) + return if sliding_window.nil? + + implementation_class = sliding_window.class + + new_window = implementation_class.new(max_size: sliding_window.max_size) + + if sliding_window.respond_to?(:size) && !sliding_window.empty? + original_data = sliding_window.instance_variable_get(:@window) + if original_data.is_a?(Array) + new_window.instance_variable_set(:@window, original_data.dup) + end + end + + new_window + end end