mirror of
https://github.com/usestrix/strix.git
synced 2026-10-12 03:47:56 +00:00
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).
This commit is contained in:
parent
7db7aa559b
commit
8b754c207e
7 changed files with 140 additions and 14 deletions
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue