From fc84ea2af9e0b56f5d8c882c4cbe7b46868f9826 Mon Sep 17 00:00:00 2001 From: Abishanan Sriranjan Date: Thu, 19 Jun 2025 10:51:22 -0400 Subject: [PATCH 1/3] add test case ensuring "consecutiveness" property of error_threshold is satisfied --- test/circuit_breaker_test.rb | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) 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 From c26eb7cdc342791c7d82967a12be02b2b062aa40 Mon Sep 17 00:00:00 2001 From: Abishanan Sriranjan Date: Wed, 25 Jun 2025 15:12:36 -0400 Subject: [PATCH 2/3] add check to clear errors on first success if error_threshold_timeout_enabled false --- lib/semian/circuit_breaker.rb | 12 ++++++--- test/circuit_breaker_test.rb | 9 +++++++ test/helpers/circuit_breaker_helper.rb | 37 ++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 4 deletions(-) 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 4a1728542..3978c1558 100644 --- a/test/circuit_breaker_test.rb +++ b/test/circuit_breaker_test.rb @@ -87,9 +87,18 @@ def test_once_success_threshold_is_reached_only_error_threshold_will_open_the_ci ) half_open_cicuit!(resource) + STDERR.puts "resource.errors: #{resource.circuit_breaker.instance_variable_get(:@errors).size}" + STDERR.puts "circuit state: #{resource.circuit_breaker.state.value}" + STDERR.puts "error timeout expired: #{resource.circuit_breaker.send(:error_timeout_expired?)}" + STDERR.puts "half open: #{resource.circuit_breaker.send(:half_open?)}" + assert_circuit_closed(resource) trigger_error!(resource) + STDERR.puts "resource.errors: #{resource.circuit_breaker.instance_variable_get(:@errors).size}" + STDERR.puts "circuit state: #{resource.circuit_breaker.state.value}" + STDERR.puts "error timeout expired: #{resource.circuit_breaker.send(:error_timeout_expired?)}" + assert_circuit_closed(resource) trigger_error!(resource) diff --git a/test/helpers/circuit_breaker_helper.rb b/test/helpers/circuit_breaker_helper.rb index c5db6f879..90af98a41 100644 --- a/test/helpers/circuit_breaker_helper.rb +++ b/test/helpers/circuit_breaker_helper.rb @@ -22,8 +22,28 @@ 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?) + + STDERR.puts "old_errors: #{old_errors ? old_errors.size : "nil"}" + STDERR.puts "new_errors: #{circuit_breaker.instance_variable_get(:@errors) ? circuit_breaker.instance_variable_get(:@errors).size : "nil"}" + + 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 + + STDERR.puts "previously_half_open: #{previously_half_open}" + STDERR.puts "error_threshold_timeout_enabled: #{circuit_breaker.instance_variable_get(:@error_threshold_timeout_enabled)}" + STDERR.puts "errors: #{circuit_breaker.instance_variable_get(:@errors) ? circuit_breaker.instance_variable_get(:@errors).size : "nil"}" + assert(block_called, "Expected the circuit to be closed, but it was open") end @@ -37,4 +57,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 From b967d3d389cc540e3be5ac9a006b5d629e95b4e4 Mon Sep 17 00:00:00 2001 From: Abishanan Sriranjan Date: Mon, 7 Jul 2025 15:29:48 -0400 Subject: [PATCH 3/3] remove debug statements --- test/circuit_breaker_test.rb | 9 --------- test/helpers/circuit_breaker_helper.rb | 7 ------- 2 files changed, 16 deletions(-) diff --git a/test/circuit_breaker_test.rb b/test/circuit_breaker_test.rb index 3978c1558..4a1728542 100644 --- a/test/circuit_breaker_test.rb +++ b/test/circuit_breaker_test.rb @@ -87,18 +87,9 @@ def test_once_success_threshold_is_reached_only_error_threshold_will_open_the_ci ) half_open_cicuit!(resource) - STDERR.puts "resource.errors: #{resource.circuit_breaker.instance_variable_get(:@errors).size}" - STDERR.puts "circuit state: #{resource.circuit_breaker.state.value}" - STDERR.puts "error timeout expired: #{resource.circuit_breaker.send(:error_timeout_expired?)}" - STDERR.puts "half open: #{resource.circuit_breaker.send(:half_open?)}" - assert_circuit_closed(resource) trigger_error!(resource) - STDERR.puts "resource.errors: #{resource.circuit_breaker.instance_variable_get(:@errors).size}" - STDERR.puts "circuit state: #{resource.circuit_breaker.state.value}" - STDERR.puts "error timeout expired: #{resource.circuit_breaker.send(:error_timeout_expired?)}" - assert_circuit_closed(resource) trigger_error!(resource) diff --git a/test/helpers/circuit_breaker_helper.rb b/test/helpers/circuit_breaker_helper.rb index 90af98a41..eb4e2914e 100644 --- a/test/helpers/circuit_breaker_helper.rb +++ b/test/helpers/circuit_breaker_helper.rb @@ -33,17 +33,10 @@ def assert_circuit_closed(resource = @resource) now_closed = !circuit_breaker&.send(:open?) - STDERR.puts "old_errors: #{old_errors ? old_errors.size : "nil"}" - STDERR.puts "new_errors: #{circuit_breaker.instance_variable_get(:@errors) ? circuit_breaker.instance_variable_get(:@errors).size : "nil"}" - 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 - STDERR.puts "previously_half_open: #{previously_half_open}" - STDERR.puts "error_threshold_timeout_enabled: #{circuit_breaker.instance_variable_get(:@error_threshold_timeout_enabled)}" - STDERR.puts "errors: #{circuit_breaker.instance_variable_get(:@errors) ? circuit_breaker.instance_variable_get(:@errors).size : "nil"}" - assert(block_called, "Expected the circuit to be closed, but it was open") end