mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-07 13:02:07 +00:00
fix(credential-pool): distinguish OpenRouter upstream 429s from account 429s
OpenRouter returns 429 in two shapes: an account-level throttle on the user's key, and an upstream-provider throttle (DeepSeek/Anthropic/etc. rate-limiting OpenRouter's aggregate traffic). The classifier treated both identically and rotated/exhausted OPENROUTER_API_KEY on every 429 — burning the key for ~24min and silently disabling auxiliary features (compression, summarization, vision) on an upstream throttle where the key was healthy. Add a FailoverReason.upstream_rate_limit classified from OpenRouter's unambiguous wrapper message "Provider returned error" (the same signal the metadata-raw parser already trusts). Recovery skips credential rotation and defers to the fallback chain to switch models instead. Co-authored-by: Hermes Agent <127238744+teknium1@users.noreply.github.com>
This commit is contained in:
parent
abca77615a
commit
ac380050ea
6 changed files with 218 additions and 8 deletions
|
|
@ -663,6 +663,25 @@ def recover_with_credential_pool(
|
|||
elif status_code in {401, 403}:
|
||||
effective_reason = FailoverReason.auth
|
||||
|
||||
if effective_reason == FailoverReason.upstream_rate_limit:
|
||||
# An upstream provider (e.g. DeepSeek behind OpenRouter) is
|
||||
# rate-limiting the aggregator's traffic — the user's credential is
|
||||
# healthy. Do NOT rotate or mark exhausted; let the caller's fallback
|
||||
# path switch to a different model entirely.
|
||||
upstream = (error_context or {}).get("upstream_provider") if error_context else None
|
||||
if upstream:
|
||||
_ra().logger.info(
|
||||
"Upstream provider %s rate-limited via aggregator — skipping "
|
||||
"credential rotation, deferring to fallback chain",
|
||||
upstream,
|
||||
)
|
||||
else:
|
||||
_ra().logger.info(
|
||||
"Upstream aggregator 429 (provider unknown) — skipping "
|
||||
"credential rotation, deferring to fallback chain"
|
||||
)
|
||||
return False, has_retried_429
|
||||
|
||||
if effective_reason == FailoverReason.billing:
|
||||
rotate_status = status_code if status_code is not None else 402
|
||||
next_entry = pool.mark_exhausted_and_rotate(status_code=rotate_status, error_context=error_context)
|
||||
|
|
|
|||
|
|
@ -1124,7 +1124,7 @@ def try_activate_fallback(agent, reason: "FailoverReason | None" = None) -> bool
|
|||
auth resolution and client construction — no duplicated provider→key
|
||||
mappings.
|
||||
"""
|
||||
if reason in {FailoverReason.rate_limit, FailoverReason.billing}:
|
||||
if reason in {FailoverReason.rate_limit, FailoverReason.billing, FailoverReason.upstream_rate_limit}:
|
||||
# Only start cooldown when leaving the primary provider. If we're
|
||||
# already on a fallback and chain-switching, the primary wasn't the
|
||||
# source of the 429 so the cooldown should not be reset/extended.
|
||||
|
|
@ -1142,7 +1142,7 @@ def try_activate_fallback(agent, reason: "FailoverReason | None" = None) -> bool
|
|||
# provider again. Guards the cross-turn replay storm in #24996.
|
||||
if (
|
||||
len(agent._fallback_chain) > 0
|
||||
and reason not in {FailoverReason.rate_limit, FailoverReason.billing}
|
||||
and reason not in {FailoverReason.rate_limit, FailoverReason.billing, FailoverReason.upstream_rate_limit}
|
||||
):
|
||||
_existing_cooldown = getattr(agent, "_rate_limited_until", 0) or 0
|
||||
agent._rate_limited_until = max(
|
||||
|
|
|
|||
|
|
@ -2920,6 +2920,7 @@ def run_conversation(
|
|||
is_rate_limited = classified.reason in {
|
||||
FailoverReason.rate_limit,
|
||||
FailoverReason.billing,
|
||||
FailoverReason.upstream_rate_limit,
|
||||
}
|
||||
_is_transport_failure = classified.reason in {
|
||||
FailoverReason.timeout,
|
||||
|
|
@ -2934,13 +2935,30 @@ def run_conversation(
|
|||
# still recover. See _pool_may_recover_from_rate_limit
|
||||
# for the single-credential-pool and CloudCode-quota
|
||||
# exceptions. Fixes #11314 and #13636.
|
||||
pool_may_recover = _ra()._pool_may_recover_from_rate_limit(
|
||||
agent._credential_pool,
|
||||
provider=agent.provider,
|
||||
base_url=getattr(agent, "base_url", None),
|
||||
#
|
||||
# Exception: an upstream-aggregator 429 — the credential
|
||||
# pool can't help when the *upstream* model (DeepSeek,
|
||||
# etc.) is throttling OpenRouter, so always fall back to a
|
||||
# different model regardless of pool state.
|
||||
_is_upstream = classified.reason == FailoverReason.upstream_rate_limit
|
||||
pool_may_recover = (
|
||||
False if _is_upstream
|
||||
else _ra()._pool_may_recover_from_rate_limit(
|
||||
agent._credential_pool,
|
||||
provider=agent.provider,
|
||||
base_url=getattr(agent, "base_url", None),
|
||||
)
|
||||
)
|
||||
if not pool_may_recover:
|
||||
if classified.reason == FailoverReason.billing:
|
||||
if _is_upstream:
|
||||
_upstream_name = (classified.error_context or {}).get(
|
||||
"upstream_provider", "aggregator"
|
||||
)
|
||||
agent._buffer_status(
|
||||
f"⚠️ Upstream {_upstream_name} rate-limited — "
|
||||
"switching to fallback model..."
|
||||
)
|
||||
elif classified.reason == FailoverReason.billing:
|
||||
agent._buffer_status(
|
||||
"⚠️ Billing or credits exhausted — switching to fallback provider..."
|
||||
)
|
||||
|
|
|
|||
|
|
@ -31,6 +31,9 @@ class FailoverReason(enum.Enum):
|
|||
# Billing / quota
|
||||
billing = "billing" # 402 or confirmed credit exhaustion — rotate immediately
|
||||
rate_limit = "rate_limit" # 429 or quota-based throttling — backoff then rotate
|
||||
# Upstream model rate-limited (aggregator 429) — fallback to a different
|
||||
# model, NOT credential rotation. The user's key is healthy.
|
||||
upstream_rate_limit = "upstream_rate_limit"
|
||||
|
||||
# Server-side
|
||||
overloaded = "overloaded" # 503/529 — provider overloaded, backoff
|
||||
|
|
@ -909,6 +912,22 @@ def _classify_by_status(
|
|||
FailoverReason.overloaded,
|
||||
retryable=True,
|
||||
)
|
||||
# Distinguish an OpenRouter-aggregator upstream 429 (an upstream model
|
||||
# like DeepSeek rate-limited OpenRouter's aggregate traffic) from an
|
||||
# account-level 429 (the user's key is actually throttled). OpenRouter
|
||||
# wraps upstream errors with the outer message "Provider returned
|
||||
# error" — the user's key is healthy, so marking it exhausted / rotating
|
||||
# is wrong and burns the key for ~24min. Fall back to a different model.
|
||||
if _is_openrouter_upstream_error(body, provider):
|
||||
upstream_provider = _extract_upstream_provider_name(body)
|
||||
ctx = {"upstream_provider": upstream_provider} if upstream_provider else {}
|
||||
return result_fn(
|
||||
FailoverReason.upstream_rate_limit,
|
||||
retryable=True,
|
||||
should_rotate_credential=False,
|
||||
should_fallback=True,
|
||||
error_context=ctx,
|
||||
)
|
||||
return result_fn(
|
||||
FailoverReason.rate_limit,
|
||||
retryable=True,
|
||||
|
|
@ -1445,3 +1464,49 @@ def _extract_message(error: Exception, body: dict) -> str:
|
|||
return msg.strip()[:500]
|
||||
# Fallback to str(error)
|
||||
return str(error)[:500]
|
||||
|
||||
|
||||
def _is_openrouter_upstream_error(body: Any, provider: str) -> bool:
|
||||
"""Detect OpenRouter's aggregator-wrapped upstream provider errors.
|
||||
|
||||
OpenRouter returns errors from upstream model providers (DeepSeek,
|
||||
Anthropic, etc.) wrapped with the outer message "Provider returned error"
|
||||
and the real error nested in ``metadata.raw``. This signal means the
|
||||
user's OpenRouter key is healthy — the upstream provider is the one that
|
||||
failed — so credential rotation is the wrong recovery.
|
||||
"""
|
||||
if not isinstance(body, dict):
|
||||
return False
|
||||
provider_lower = (provider or "").strip().lower()
|
||||
err = body.get("error")
|
||||
if not isinstance(err, dict):
|
||||
return False
|
||||
outer_msg = str(err.get("message") or "").strip().lower()
|
||||
if outer_msg != "provider returned error":
|
||||
return False
|
||||
# Require either the explicit OpenRouter provider OR the metadata shape
|
||||
# that only OpenRouter produces (metadata.raw / metadata.provider_name).
|
||||
if provider_lower == "openrouter":
|
||||
return True
|
||||
metadata = err.get("metadata")
|
||||
if isinstance(metadata, dict) and (
|
||||
"raw" in metadata or "provider_name" in metadata
|
||||
):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _extract_upstream_provider_name(body: Any) -> Optional[str]:
|
||||
"""Pull the upstream provider name out of OpenRouter's error metadata."""
|
||||
if not isinstance(body, dict):
|
||||
return None
|
||||
err = body.get("error")
|
||||
if not isinstance(err, dict):
|
||||
return None
|
||||
metadata = err.get("metadata")
|
||||
if not isinstance(metadata, dict):
|
||||
return None
|
||||
name = metadata.get("provider_name")
|
||||
if isinstance(name, str) and name.strip():
|
||||
return name.strip()
|
||||
return None
|
||||
|
|
|
|||
|
|
@ -50,6 +50,7 @@ AUTHOR_MAP = {
|
|||
"phanvanhoa@gmail.com": "theAgenticBuilder", # PR #14180 salvage (route delegate_task progress lines through _safe_print so ACP stdio JSON-RPC frames stay clean)
|
||||
"huangxudong663@gmail.com": "huangxudong663-sys", # PR #15157 salvage (isinstance(dict) guard on tool-call model_extra; NVIDIA NIM non-dict crash)
|
||||
"39369769+jasonQin6@users.noreply.github.com": "jasonQin6", # PR #15093 salvage (session staleness guard on stream consumer run() loop; #11016 follow-up)
|
||||
"znding04@gmail.com": "znding04", # PR #15487 salvage (distinguish OpenRouter upstream 429 from account 429; upstream_rate_limit failover reason)
|
||||
"cypher@augmentl.com": "Nickperillo", # PR #8008 salvage (Discord channel-name matching + flush pending sends on shutdown)
|
||||
"tenoryang@outlook.com": "MarioYounger", # PR #9028 salvage (bash/sh heredoc approval, NFKC homograph fold, execute_code CREDS/BEARER/APIKEY env filter)
|
||||
"peet.wannasarnmetha@gmail.com": "peetwan", # PR #51841 salvage (loopback ws-ping tuning + token-frame coalescing + loop heartbeat; #48445/#50005)
|
||||
|
|
|
|||
|
|
@ -53,6 +53,7 @@ class TestFailoverReason:
|
|||
def test_enum_members_exist(self):
|
||||
expected = {
|
||||
"auth", "auth_permanent", "billing", "rate_limit",
|
||||
"upstream_rate_limit",
|
||||
"overloaded", "server_error", "timeout",
|
||||
"context_overflow", "payload_too_large", "image_too_large",
|
||||
"model_not_found", "format_error",
|
||||
|
|
@ -1716,4 +1717,110 @@ class TestMultimodalToolContentUnsupported:
|
|||
"""Make sure the patterns don't false-positive on normal 400s."""
|
||||
e = MockAPIError("bad request: missing field 'model'", status_code=400)
|
||||
result = classify_api_error(e, provider="openrouter", model="anthropic/claude-sonnet-4")
|
||||
assert result.reason != FailoverReason.multimodal_tool_content_unsupported
|
||||
|
||||
|
||||
class TestOpenRouterUpstreamRateLimit:
|
||||
"""Distinguish upstream-provider 429 from account-level 429 on OpenRouter.
|
||||
|
||||
When an upstream model (DeepSeek, Anthropic, etc.) rate-limits OpenRouter's
|
||||
aggregate traffic, OpenRouter returns 429 with the outer message "Provider
|
||||
returned error". The user's key is healthy — we must fall back to a
|
||||
different model, NOT mark the credential exhausted.
|
||||
"""
|
||||
|
||||
def test_openrouter_upstream_429_classified_as_upstream_rate_limit(self):
|
||||
"""OpenRouter 429 with 'Provider returned error' → upstream_rate_limit."""
|
||||
e = MockAPIError(
|
||||
"Provider returned error",
|
||||
status_code=429,
|
||||
body={
|
||||
"error": {
|
||||
"message": "Provider returned error",
|
||||
"code": 429,
|
||||
"metadata": {
|
||||
"provider_name": "DeepSeek",
|
||||
"raw": '{"error":{"message":"Rate limit exceeded"}}',
|
||||
},
|
||||
}
|
||||
},
|
||||
)
|
||||
result = classify_api_error(e, provider="openrouter", model="deepseek/deepseek-v4-flash")
|
||||
assert result.reason == FailoverReason.upstream_rate_limit
|
||||
assert result.should_rotate_credential is False
|
||||
assert result.should_fallback is True
|
||||
assert result.error_context.get("upstream_provider") == "DeepSeek"
|
||||
|
||||
def test_upstream_429_metadata_shape_without_explicit_provider(self):
|
||||
"""metadata.raw shape alone (provider != openrouter literal) still detected."""
|
||||
e = MockAPIError(
|
||||
"Provider returned error",
|
||||
status_code=429,
|
||||
body={
|
||||
"error": {
|
||||
"message": "Provider returned error",
|
||||
"metadata": {"raw": '{"error":{"code":429}}'},
|
||||
}
|
||||
},
|
||||
)
|
||||
# provider passed as the slug-form some callers use
|
||||
result = classify_api_error(e, provider="openrouter", model="x")
|
||||
assert result.reason == FailoverReason.upstream_rate_limit
|
||||
|
||||
def test_account_level_429_still_rotates_credential(self):
|
||||
"""A real account-level 429 (no upstream wrapper) → rate_limit, rotates."""
|
||||
e = MockAPIError(
|
||||
"Rate limit exceeded: 200 requests per minute",
|
||||
status_code=429,
|
||||
body={
|
||||
"error": {
|
||||
"message": "Rate limit exceeded: 200 requests per minute",
|
||||
"code": 429,
|
||||
}
|
||||
},
|
||||
)
|
||||
result = classify_api_error(e, provider="openrouter", model="deepseek/deepseek-v4-flash")
|
||||
assert result.reason == FailoverReason.rate_limit
|
||||
assert result.should_rotate_credential is True
|
||||
|
||||
def test_upstream_wrapper_without_metadata_on_non_openrouter_not_matched(self):
|
||||
"""'Provider returned error' alone on a non-openrouter provider → plain rate_limit."""
|
||||
e = MockAPIError(
|
||||
"Provider returned error",
|
||||
status_code=429,
|
||||
body={"error": {"message": "Provider returned error", "code": 429}},
|
||||
)
|
||||
result = classify_api_error(e, provider="anthropic", model="claude-sonnet-4")
|
||||
assert result.reason == FailoverReason.rate_limit
|
||||
|
||||
def test_upstream_provider_name_missing_yields_empty_context(self):
|
||||
"""No provider_name in metadata → upstream_rate_limit with empty context."""
|
||||
e = MockAPIError(
|
||||
"Provider returned error",
|
||||
status_code=429,
|
||||
body={
|
||||
"error": {
|
||||
"message": "Provider returned error",
|
||||
"metadata": {"raw": '{"error":{"code":429}}'},
|
||||
}
|
||||
},
|
||||
)
|
||||
result = classify_api_error(e, provider="openrouter", model="x")
|
||||
assert result.reason == FailoverReason.upstream_rate_limit
|
||||
assert result.error_context.get("upstream_provider") is None
|
||||
|
||||
def test_overload_429_takes_precedence_over_upstream(self):
|
||||
"""A 429 carrying overload language stays overloaded (retry same key)."""
|
||||
e = MockAPIError(
|
||||
"Provider returned error",
|
||||
status_code=429,
|
||||
body={
|
||||
"error": {
|
||||
"message": "service is temporarily overloaded",
|
||||
"metadata": {"provider_name": "DeepSeek"},
|
||||
}
|
||||
},
|
||||
)
|
||||
result = classify_api_error(e, provider="openrouter", model="x")
|
||||
# Overload disambiguation runs first; the outer message is the overload
|
||||
# phrase, so this is an overload, not an upstream rate-limit.
|
||||
assert result.reason == FailoverReason.overloaded
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue