From 79611673d776cf37fd1d8bb30b61536c3efec2db Mon Sep 17 00:00:00 2001 From: Daoyuan Li <94409450+DaoyuanLi2816@users.noreply.github.com> Date: Fri, 10 Jul 2026 16:54:15 -0700 Subject: [PATCH] Fix circuit breaker wedging after a non-retriable half-open probe (#3991) * Fix circuit breaker wedging after a non-retriable half-open probe When the circuit breaker is half-open it admits a single probe call by setting `_circuit_probe_in_flight = True`. If that probe raised a *non-retriable* error (e.g. quota/auth), the except block skipped both `_record_failure()` (correct - business errors must not trip the breaker) and any probe reset, so the circuit stayed `half_open` with `_circuit_probe_in_flight = True` permanently. Every later call then fast-failed in `_check_circuit()` forever, because no call could run the handler to reach `_record_success` / `_record_failure`. Release the probe on the non-retriable path (mirroring the existing GraphBubbleUp handler) so the next call admits a fresh probe. The breaker still never trips on non-retriable errors. Applied to both the sync and async paths. Adds sync + async regression tests asserting the probe is released and the next `_check_circuit()` re-admits a probe. * Address review: extract _release_half_open_probe helper --- .../llm_error_handling_middleware.py | 25 +++++-- .../test_llm_error_handling_middleware.py | 67 +++++++++++++++++++ 2 files changed, 86 insertions(+), 6 deletions(-) diff --git a/backend/packages/harness/deerflow/agents/middlewares/llm_error_handling_middleware.py b/backend/packages/harness/deerflow/agents/middlewares/llm_error_handling_middleware.py index 78ae81589..71c00f4aa 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/llm_error_handling_middleware.py +++ b/backend/packages/harness/deerflow/agents/middlewares/llm_error_handling_middleware.py @@ -182,6 +182,17 @@ class LLMErrorHandlingMiddleware(AgentMiddleware[AgentState]): self.circuit_recovery_timeout_sec, ) + def _release_half_open_probe(self) -> None: + """Release the in-flight half-open probe without recording a failure. + + Used when something other than a classified success/failure consumes the probe (a + GraphBubbleUp control-flow signal, or a non-retriable error), so the circuit can admit + the next probe instead of fast-failing forever. + """ + with self._circuit_lock: + if self._circuit_state == "half_open": + self._circuit_probe_in_flight = False + def _classify_error(self, exc: BaseException) -> tuple[bool, str]: detail = _extract_error_detail(exc) lowered = detail.lower() @@ -326,9 +337,7 @@ class LLMErrorHandlingMiddleware(AgentMiddleware[AgentState]): return response except GraphBubbleUp: # Preserve LangGraph control-flow signals (interrupt/pause/resume). - with self._circuit_lock: - if self._circuit_state == "half_open": - self._circuit_probe_in_flight = False + self._release_half_open_probe() raise except Exception as exc: retriable, reason = self._classify_error(exc) @@ -354,6 +363,9 @@ class LLMErrorHandlingMiddleware(AgentMiddleware[AgentState]): ) if retriable: self._record_failure() + else: + # Non-retriable: release the probe without recording a failure. + self._release_half_open_probe() return self._build_user_fallback_message(exc, reason) @override @@ -378,9 +390,7 @@ class LLMErrorHandlingMiddleware(AgentMiddleware[AgentState]): return response except GraphBubbleUp: # Preserve LangGraph control-flow signals (interrupt/pause/resume). - with self._circuit_lock: - if self._circuit_state == "half_open": - self._circuit_probe_in_flight = False + self._release_half_open_probe() raise except Exception as exc: retriable, reason = self._classify_error(exc) @@ -406,6 +416,9 @@ class LLMErrorHandlingMiddleware(AgentMiddleware[AgentState]): ) if retriable: self._record_failure() + else: + # Non-retriable: release the probe without recording a failure. + self._release_half_open_probe() return self._build_user_fallback_message(exc, reason) diff --git a/backend/tests/test_llm_error_handling_middleware.py b/backend/tests/test_llm_error_handling_middleware.py index 3fe1e36bf..016c53a96 100644 --- a/backend/tests/test_llm_error_handling_middleware.py +++ b/backend/tests/test_llm_error_handling_middleware.py @@ -224,6 +224,73 @@ async def test_async_circuit_half_open_graph_bubble_up_resets_probe() -> None: assert middleware._circuit_state == "half_open" +def test_circuit_half_open_non_retriable_error_resets_probe() -> None: + """A non-retriable error during a half-open probe must release the probe. + + Regression: the non-retriable branch neither recorded a failure (correct — + business errors like quota/auth must not trip the breaker) nor reset + ``_circuit_probe_in_flight``. So one non-retriable probe left the circuit + stuck at half_open with probe_in_flight=True, and every subsequent call + fast-failed forever because no later call could ever run the handler to + reach ``_record_success`` / ``_record_failure``. + """ + import unittest.mock + + middleware = _build_middleware() + + # Enter half_open and let one probe through (probe_in_flight -> True). + middleware._circuit_state = "half_open" + middleware._circuit_probe_in_flight = False + assert middleware._check_circuit() is False + assert middleware._circuit_probe_in_flight is True + + def handler(_request) -> AIMessage: + raise FakeError("insufficient_quota", status_code=429, code="insufficient_quota") + + # _check_circuit already admitted the probe above; keep it False here so the + # top-of-call gate does not fast-fail before the handler runs. Force the + # error to classify as non-retriable regardless of heuristics. + with unittest.mock.patch.object(middleware, "_check_circuit", return_value=False): + with unittest.mock.patch.object(middleware, "_classify_error", return_value=(False, "quota")): + result = middleware.wrap_model_call(SimpleNamespace(), handler) + + # Non-retriable errors still surface a graceful fallback (not a raise) and + # must NOT trip the breaker. + assert isinstance(result, AIMessage) + assert middleware._circuit_state == "half_open" + # The probe was released, so the real gate re-admits the next probe instead + # of fast-failing forever. + assert middleware._circuit_probe_in_flight is False + assert middleware._check_circuit() is False + assert middleware._circuit_probe_in_flight is True + + +@pytest.mark.anyio +async def test_async_circuit_half_open_non_retriable_error_resets_probe() -> None: + """Async mirror: a non-retriable error during a half-open probe releases it.""" + import unittest.mock + + middleware = _build_middleware() + + middleware._circuit_state = "half_open" + middleware._circuit_probe_in_flight = False + assert middleware._check_circuit() is False + assert middleware._circuit_probe_in_flight is True + + async def handler(_request) -> AIMessage: + raise FakeError("insufficient_quota", status_code=429, code="insufficient_quota") + + with unittest.mock.patch.object(middleware, "_check_circuit", return_value=False): + with unittest.mock.patch.object(middleware, "_classify_error", return_value=(False, "quota")): + result = await middleware.awrap_model_call(SimpleNamespace(), handler) + + assert isinstance(result, AIMessage) + assert middleware._circuit_state == "half_open" + assert middleware._circuit_probe_in_flight is False + assert middleware._check_circuit() is False + assert middleware._circuit_probe_in_flight is True + + # ---------- Circuit Breaker Tests ----------