fix(auxiliary): fall back on 401 auth errors in auto mode (#21165)
When the primary provider returns 401 and the auth-refresh path is unavailable or fails, both call_llm() and async_call_llm() reached the should_fallback gate without _is_auth_error in the condition, so the auxiliary task (e.g. compression) was dropped silently — losing message history. Add _is_auth_error to should_fallback (NOT is_capacity_error) in both sync and async paths, plus an 'auth error' reason branch. Auth stays a non-capacity error: it falls back in auto mode via the is_auto gate, but on an explicitly-configured provider it still respects the user's choice and raises rather than silently switching providers.
This commit is contained in:
parent
1a570dae00
commit
46e18804ad
2 changed files with 78 additions and 4 deletions
|
|
@ -5993,8 +5993,17 @@ def call_llm(
|
|||
# When the provider returns a 429 rate-limit (not billing), fall
|
||||
# back to an alternative provider instead of exhausting retries
|
||||
# against the same rate-limited endpoint.
|
||||
#
|
||||
# ── Auth error fallback (#21165) ─────────────────────────────
|
||||
# When the resolved provider returns 401 and neither the Nous
|
||||
# refresh path nor explicit provider credential refresh applies,
|
||||
# fall back to an alternative provider instead of dropping the
|
||||
# auxiliary task on the floor (silent compression failure /
|
||||
# message loss). Auth is NOT a capacity error: it only bypasses
|
||||
# the explicit-provider gate when the user is in auto mode.
|
||||
should_fallback = (
|
||||
_is_payment_error(first_err)
|
||||
_is_auth_error(first_err)
|
||||
or _is_payment_error(first_err)
|
||||
or _is_connection_error(first_err)
|
||||
or _is_rate_limit_error(first_err)
|
||||
or _is_model_incompatible_error(first_err)
|
||||
|
|
@ -6024,7 +6033,9 @@ def call_llm(
|
|||
or _is_invalid_aux_response_error(first_err)
|
||||
)
|
||||
if should_fallback and (is_auto or is_capacity_error):
|
||||
if _is_payment_error(first_err):
|
||||
if _is_auth_error(first_err):
|
||||
reason = "auth error"
|
||||
elif _is_payment_error(first_err):
|
||||
reason = "payment error"
|
||||
# Resolve the actual provider label (resolved_provider may be
|
||||
# "auto"; the client's base_url tells us which backend got the
|
||||
|
|
@ -6473,8 +6484,13 @@ async def async_call_llm(
|
|||
raise
|
||||
|
||||
# ── Payment / connection / rate-limit fallback (mirrors sync call_llm) ──
|
||||
# Auth error fallback (#21165): a 401 that survived the refresh path
|
||||
# falls back in auto mode just like the sync call_llm() path. Auth is
|
||||
# NOT a capacity error, so on an explicit provider it still respects
|
||||
# the user's choice (handled by the is_auto/is_capacity_error gate).
|
||||
should_fallback = (
|
||||
_is_payment_error(first_err)
|
||||
_is_auth_error(first_err)
|
||||
or _is_payment_error(first_err)
|
||||
or _is_connection_error(first_err)
|
||||
or _is_rate_limit_error(first_err)
|
||||
or _is_model_incompatible_error(first_err)
|
||||
|
|
@ -6496,7 +6512,9 @@ async def async_call_llm(
|
|||
or _is_invalid_aux_response_error(first_err)
|
||||
)
|
||||
if should_fallback and (is_auto or is_capacity_error):
|
||||
if _is_payment_error(first_err):
|
||||
if _is_auth_error(first_err):
|
||||
reason = "auth error"
|
||||
elif _is_payment_error(first_err):
|
||||
reason = "payment error"
|
||||
_mark_provider_unhealthy(
|
||||
_recoverable_pool_provider(resolved_provider, client) or resolved_provider
|
||||
|
|
|
|||
|
|
@ -1842,6 +1842,62 @@ class TestCallLlmPaymentFallback:
|
|||
# Fallback client should have been used
|
||||
assert fallback_client.chat.completions.create.called
|
||||
|
||||
def test_401_auth_error_triggers_fallback_in_auto_mode(self, monkeypatch):
|
||||
"""401 auth errors should trigger fallback in auto mode (#21165).
|
||||
|
||||
When refresh is unavailable/fails and the user is on the auto chain,
|
||||
a 401 must fall back instead of silently dropping the aux task
|
||||
(which caused compression message loss).
|
||||
"""
|
||||
monkeypatch.setenv("OPENROUTER_API_KEY", "or-key")
|
||||
|
||||
primary_client = MagicMock()
|
||||
primary_client.base_url = "https://api.minimax.chat/v1"
|
||||
primary_client.chat.completions.create.side_effect = _AuxAuth401("expired key")
|
||||
|
||||
fallback_client = MagicMock()
|
||||
fallback_client.chat.completions.create.return_value = _DummyResponse("fallback auth response")
|
||||
|
||||
with patch("agent.auxiliary_client._get_cached_client",
|
||||
return_value=(primary_client, "minimax/minimax-m2.7")), \
|
||||
patch("agent.auxiliary_client._resolve_task_provider_model",
|
||||
return_value=("auto", "minimax/minimax-m2.7", None, None, None)), \
|
||||
patch("agent.auxiliary_client._try_payment_fallback",
|
||||
return_value=(fallback_client, "fallback-model", "openrouter")) as mock_fb:
|
||||
result = call_llm(
|
||||
task="compression",
|
||||
messages=[{"role": "user", "content": "hello"}],
|
||||
)
|
||||
|
||||
assert result.choices[0].message.content == "fallback auth response"
|
||||
assert fallback_client.chat.completions.create.called
|
||||
# Labelled as an auth error, not mis-tagged as a connection error.
|
||||
assert mock_fb.call_args.kwargs.get("reason") == "auth error"
|
||||
|
||||
def test_401_auth_error_no_fallback_with_explicit_provider(self, monkeypatch):
|
||||
"""401 on an explicitly-configured provider must NOT silently switch.
|
||||
|
||||
Auth is not a capacity error: the explicit-provider gate means a 401
|
||||
respects the user's choice and raises instead of falling back. This
|
||||
guards the deliberate design at the should_fallback/is_capacity gate.
|
||||
"""
|
||||
primary_client = MagicMock()
|
||||
primary_client.base_url = "https://api.minimax.chat/v1"
|
||||
primary_client.chat.completions.create.side_effect = _AuxAuth401("expired key")
|
||||
|
||||
with patch("agent.auxiliary_client._get_cached_client",
|
||||
return_value=(primary_client, "minimax/minimax-m2.7")), \
|
||||
patch("agent.auxiliary_client._resolve_task_provider_model",
|
||||
return_value=("minimax", "minimax/minimax-m2.7", None, None, None)), \
|
||||
patch("agent.auxiliary_client._refresh_provider_credentials", return_value=False), \
|
||||
patch("agent.auxiliary_client._try_payment_fallback") as mock_fb:
|
||||
with pytest.raises(_AuxAuth401):
|
||||
call_llm(
|
||||
task="compression",
|
||||
messages=[{"role": "user", "content": "hello"}],
|
||||
)
|
||||
mock_fb.assert_not_called()
|
||||
|
||||
|
||||
class TestAuxiliaryFallbackLayering:
|
||||
"""Explicit-provider users get layered fallback: configured_chain → main agent → warn."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue