From 8b754c207ea98fb6156310652f618fa60d3bf1f8 Mon Sep 17 00:00:00 2001 From: Ian Date: Tue, 6 Oct 2026 15:26:50 -0400 Subject: [PATCH] feat(llm): retry upstream 4xx errors and ban the provider that keeps failing LiteLLM does not retry 4xx and OpenRouter does not fall back on a provider's 400 or after the stream starts, so one bad upstream (Relace's "incomplete tool call", InferenceNet's 250k context cap) killed the agent. Every 4xx is now replayed within the existing 5 retries, except guardrail refusals and the gateway's own 401/402/403. Before each replay the upstream that failed is banned for the model: right away on provider_unavailable or when OpenRouter named it in a non-2xx reply, and after STRIX_PROVIDER_4XX_BAN_AFTER (default 2) mid-stream 4xx errors. RequestLoggingModel attaches the attempt's HttpReply to the exception it raises, so the run cycle can read the upstream provider with request_log.failed_reply(exc). --- strix/config/models.py | 4 +- strix/config/settings.py | 1 + strix/core/execution.py | 36 +++++++++++- strix/llm/request_log.py | 16 ++++- tests/test_cost_tracking.py | 2 + tests/test_execution_transient_retry.py | 78 +++++++++++++++++++++---- tests/test_llm_request_log.py | 17 ++++++ 7 files changed, 140 insertions(+), 14 deletions(-) diff --git a/strix/config/models.py b/strix/config/models.py index 6a0b8976..063794fd 100644 --- a/strix/config/models.py +++ b/strix/config/models.py @@ -774,7 +774,9 @@ def _install_openrouter_stream_cost_capture() -> None: with contextlib.suppress(Exception): error = json.loads(error_message)["error"] request_log.record_upstream_provider( - error["metadata"].get("provider_name"), _openrouter_error_type(error) + error["metadata"].get("provider_name"), + _openrouter_error_type(error), + rejected=True, ) return super().get_error_class(error_message, status_code, headers) diff --git a/strix/config/settings.py b/strix/config/settings.py index 340f05b3..a471286e 100644 --- a/strix/config/settings.py +++ b/strix/config/settings.py @@ -73,6 +73,7 @@ class LlmSettings(BaseSettings): alias="STRIX_OPENROUTER_STICKY_SESSIONS", ) openrouter_provider_bans: bool = Field(default=True, alias="STRIX_OPENROUTER_PROVIDER_BANS") + openrouter_ban_after_4xx: int = Field(default=2, ge=1, alias="STRIX_OPENROUTER_BAN_AFTER_4XX") disable_streaming: bool = Field( default=False, alias="LLM_DISABLE_STREAMING", diff --git a/strix/core/execution.py b/strix/core/execution.py index 91d94498..c1d9b621 100644 --- a/strix/core/execution.py +++ b/strix/core/execution.py @@ -6,6 +6,7 @@ import asyncio import contextlib import logging import uuid +from collections import Counter from collections.abc import Callable from functools import cache from typing import TYPE_CHECKING, Any, cast @@ -20,6 +21,7 @@ from openai import ( ) from strix.config import codex +from strix.config.loader import load_settings from strix.core.hooks import ( BudgetExceededError, BudgetPausedError, @@ -33,7 +35,7 @@ from strix.core.sessions import ( seed_initial_input, strip_all_images_from_session, ) -from strix.llm import request_log +from strix.llm import provider_bans, request_log from strix.llm.compaction import is_context_overflow, maybe_compact @@ -123,6 +125,7 @@ async def _compact_session( _MAX_TRANSIENT_MODEL_RETRIES = 5 _TRANSIENT_MODEL_RETRY_BASE_DELAY_S = 2.0 _TRANSIENT_MODEL_RETRY_MAX_DELAY_S = 90.0 +_provider_4xx_counts: Counter[tuple[str, str]] = Counter() def _model_error_status_code(exc: BaseException) -> int | None: @@ -138,13 +141,41 @@ def _is_transient_model_error(exc: BaseException) -> bool: ): return True code = _model_error_status_code(exc) + if code in (401, 402, 403): + # Auth, credit and policy errors from the gateway itself (e.g. OpenRouter's + # "User not found" 401) name no upstream provider; retrying cannot help. + reply = request_log.failed_reply(exc) + return reply is not None and reply.upstream_provider is not None if code is not None: import litellm - return bool(litellm._should_retry(code)) + return 400 <= code < 500 or bool(litellm._should_retry(code)) return isinstance(exc, APIError) +async def _maybe_ban_provider(exc: BaseException, run_config: RunConfig) -> None: + reply = request_log.failed_reply(exc) + if reply is None or reply.upstream_provider is None: + return + model = _run_config_model(run_config) + if model is None: + return + provider = reply.upstream_provider + code = _model_error_status_code(exc) + if reply.upstream_error_type == "provider_unavailable": + reason = "provider_unavailable" + elif reply.upstream_rejected: + reason = f"upstream_rejected_{code}" + elif code is not None and 400 <= code < 500: + _provider_4xx_counts[model, provider] += 1 + if _provider_4xx_counts[model, provider] < load_settings().llm.openrouter_ban_after_4xx: + return + reason = "repeated_4xx" + else: + return + await asyncio.to_thread(provider_bans.ban_provider, model, provider, reason) + + def _transient_model_retry_delay(attempt: int) -> float: delay = _TRANSIENT_MODEL_RETRY_BASE_DELAY_S * float(2 ** (attempt - 1)) return min(delay, _TRANSIENT_MODEL_RETRY_MAX_DELAY_S) @@ -844,6 +875,7 @@ async def _run_cycle( # noqa: PLR0912, PLR0915 input_data = [] continue if model_retries < _MAX_TRANSIENT_MODEL_RETRIES and _is_transient_model_error(exc): + await _maybe_ban_provider(exc, run_config) model_retries += 1 delay = _transient_model_retry_delay(model_retries) logger.warning( diff --git a/strix/llm/request_log.py b/strix/llm/request_log.py index 8e55a736..99f313e1 100644 --- a/strix/llm/request_log.py +++ b/strix/llm/request_log.py @@ -161,18 +161,24 @@ class HttpReply: headers: dict[str, str] | None = None upstream_provider: str | None = None upstream_error_type: str | None = None + # The provider rejected the request (named in the gateway's error body). + upstream_rejected: bool = False _http_reply: ContextVar[HttpReply | None] = ContextVar("strix_llm_http_reply", default=None) +_FAILED_REPLY_ATTR = "_strix_http_reply" -def record_upstream_provider(provider: object, error_type: object = None) -> None: +def record_upstream_provider( + provider: object, error_type: object = None, *, rejected: bool = False +) -> None: """Remember which provider behind a gateway served the attempt in flight.""" reply = _http_reply.get() if reply is None: return if isinstance(provider, str) and provider: reply.upstream_provider = provider + reply.upstream_rejected = rejected if isinstance(error_type, str) and error_type: reply.upstream_error_type = error_type @@ -186,6 +192,12 @@ def _upstream_details(reply: HttpReply | None) -> list[tuple[str, object]]: ] +def failed_reply(exc: BaseException) -> HttpReply | None: + """The reply of the attempt that raised ``exc``.""" + reply = getattr(exc, _FAILED_REPLY_ATTR, None) + return reply if isinstance(reply, HttpReply) else None + + async def record_http_reply(response: Response) -> None: """httpx ``response`` event hook: remember the reply for the attempt in flight. @@ -1207,6 +1219,7 @@ class RequestLoggingModel(Model): prompt=prompt, ) except BaseException as exc: + setattr(exc, _FAILED_REPLY_ATTR, reply) if self._should_emit(exc): emit( self._event( @@ -1301,6 +1314,7 @@ class RequestLoggingModel(Model): raw_response = event.response yield event except BaseException as exc: + setattr(exc, _FAILED_REPLY_ATTR, reply) if self._should_emit(exc): emit( self._event( diff --git a/tests/test_cost_tracking.py b/tests/test_cost_tracking.py index cae4a0a4..3f50ebeb 100644 --- a/tests/test_cost_tracking.py +++ b/tests/test_cost_tracking.py @@ -466,6 +466,7 @@ def test_openrouter_records_upstream_of_stream_and_mid_stream_error() -> None: request_log._http_reply.reset(token) assert reply.upstream_provider == "Relace" assert reply.upstream_error_type == "invalid_request" + assert reply.upstream_rejected is False def test_openrouter_records_upstream_of_pre_stream_error() -> None: @@ -485,3 +486,4 @@ def test_openrouter_records_upstream_of_pre_stream_error() -> None: request_log._http_reply.reset(token) assert reply.upstream_provider == "InferenceNet" assert reply.upstream_error_type is None + assert reply.upstream_rejected is True diff --git a/tests/test_execution_transient_retry.py b/tests/test_execution_transient_retry.py index d81e4e70..aa43fb50 100644 --- a/tests/test_execution_transient_retry.py +++ b/tests/test_execution_transient_retry.py @@ -1,6 +1,7 @@ from __future__ import annotations -from typing import Any, cast +from collections import Counter +from typing import Any import httpx import pytest @@ -18,6 +19,7 @@ from openai import ( from strix.config import codex from strix.core import execution from strix.core.agents import AgentCoordinator +from strix.llm import provider_bans, request_log def _request() -> httpx.Request: @@ -79,15 +81,74 @@ def test_content_guardrail_is_not_retried() -> None: assert execution._is_transient_model_error(guardrail) is False -def test_client_errors_are_not_transient() -> None: +def _upstream_error( + status: int, provider: str, *, error_type: str | None = None, rejected: bool = False +) -> APIStatusError: + exc = _status_error(status) + reply = request_log.HttpReply( + upstream_provider=provider, upstream_error_type=error_type, upstream_rejected=rejected + ) + setattr(exc, request_log._FAILED_REPLY_ATTR, reply) + return exc + + +def test_client_errors_are_transient() -> None: bad_request = BadRequestError( "bad", response=httpx.Response(400, request=_request()), body=None ) - assert execution._is_transient_model_error(bad_request) is False - assert execution._is_transient_model_error(_status_error(404)) is False + assert execution._is_transient_model_error(bad_request) is True + assert execution._is_transient_model_error(_status_error(404)) is True assert execution._is_transient_model_error(ValueError("nope")) is False +def test_gateway_auth_credit_and_policy_errors_are_not_transient() -> None: + for status in (401, 402, 403): + assert execution._is_transient_model_error(_status_error(status)) is False + assert execution._is_transient_model_error(_upstream_error(401, "Relace")) is True + + +@pytest.fixture +def bans(monkeypatch: pytest.MonkeyPatch) -> list[tuple[str, str, str]]: + calls: list[tuple[str, str, str]] = [] + + def _ban(model: str, provider: str, reason: str) -> None: + calls.append((model, provider, reason)) + + monkeypatch.setattr(provider_bans, "ban_provider", _ban) + monkeypatch.setattr(execution, "_provider_4xx_counts", Counter()) + return calls + + +_OPENROUTER_RUN_CONFIG = RunConfig(model="openrouter/z-ai/glm-5.3") + + +@pytest.mark.asyncio +async def test_bans_upstream_right_away_on_provider_unavailable_or_rejection( + bans: list[tuple[str, str, str]], +) -> None: + unavailable = _upstream_error(502, "Relace", error_type="provider_unavailable") + await execution._maybe_ban_provider(unavailable, _OPENROUTER_RUN_CONFIG) + rejected = _upstream_error(400, "InferenceNet", rejected=True) + await execution._maybe_ban_provider(rejected, _OPENROUTER_RUN_CONFIG) + assert bans == [ + ("openrouter/z-ai/glm-5.3", "Relace", "provider_unavailable"), + ("openrouter/z-ai/glm-5.3", "InferenceNet", "upstream_rejected_400"), + ] + + +@pytest.mark.asyncio +async def test_bans_upstream_after_repeated_mid_stream_4xx( + bans: list[tuple[str, str, str]], +) -> None: + for _ in range(2): + await execution._maybe_ban_provider(_upstream_error(400, "Relace"), _OPENROUTER_RUN_CONFIG) + await execution._maybe_ban_provider( + _upstream_error(504, "Together"), _OPENROUTER_RUN_CONFIG + ) + await execution._maybe_ban_provider(_status_error(400), _OPENROUTER_RUN_CONFIG) + assert bans == [("openrouter/z-ai/glm-5.3", "Relace", "repeated_4xx")] + + class _FakeStream: def __init__(self, exc: BaseException | None = None) -> None: self._exc = exc @@ -128,7 +189,7 @@ async def _run_once( coordinator, "root", input_data="task", - run_config=cast("RunConfig", object()), + run_config=RunConfig(), context={}, max_turns=5, session=None, @@ -166,9 +227,6 @@ async def test_run_cycle_gives_up_after_max_retries( async def test_run_cycle_does_not_retry_permanent_error( monkeypatch: pytest.MonkeyPatch, ) -> None: - bad_request = BadRequestError( - "bad", response=httpx.Response(400, request=_request()), body=None - ) - streams = [_FakeStream(exc=bad_request), _FakeStream()] - with pytest.raises(BadRequestError): + streams = [_FakeStream(exc=_status_error(401)), _FakeStream()] + with pytest.raises(APIStatusError): await _run_once(monkeypatch, streams) diff --git a/tests/test_llm_request_log.py b/tests/test_llm_request_log.py index 41bbc884..0787ac3e 100644 --- a/tests/test_llm_request_log.py +++ b/tests/test_llm_request_log.py @@ -1235,6 +1235,23 @@ async def test_litellm_route_wrapper_only_logs_abandoned_attempts( assert event.request_bytes is not None +@pytest.mark.asyncio +async def test_failed_attempt_reply_rides_on_its_exception() -> None: + exc = _openai_status_error(400, None) + model = request_log.RequestLoggingModel( + _Inner(exc=exc), + model_name="openrouter/z-ai/glm-5.3", + provider="openrouter", + base_url=None, + route="litellm", + abandoned_only=True, + ) + with pytest.raises(APIStatusError): + await model.get_response(*_CALL_ARGS, **_CALL_KWARGS) + assert isinstance(request_log.failed_reply(exc), request_log.HttpReply) + assert request_log.failed_reply(ValueError()) is None + + @pytest.mark.asyncio async def test_stream_closed_early_is_logged_as_abandoned(captured: list[LlmRequestEvent]) -> None: created = ResponseCreatedEvent(