Skip to content

fix(gateway): classify transient provider-resolution errors and label fallback by config provider - #32881

Closed
konsisumer wants to merge 1 commit into
NousResearch:mainfrom
konsisumer:fix/gateway-classify-transient-provider-errors
Closed

fix(gateway): classify transient provider-resolution errors and label fallback by config provider#32881
konsisumer wants to merge 1 commit into
NousResearch:mainfrom
konsisumer:fix/gateway-classify-transient-provider-errors

Conversation

@konsisumer

Copy link
Copy Markdown
Contributor

What does this PR do?

When the primary provider's credential resolution fails, gateway.run._resolve_runtime_agent_kwargs caught every AuthError and logged it as Primary provider auth failed: ... — trying fallback. That wording is correct for a genuine credential problem (expired/revoked/missing token), but AuthError also carries transient upstream failures — a 429 usage-limit / rate-limit or a 5xx that a perfectly valid credential simply hit. Those carry relogin_required=False, yet were still surfaced as "auth failed", driving operators to re-run hermes auth on tokens that are actually fine (issue #32790, surface #2).

This branches the warning wording on the structured relogin_required hint the AuthError already exposes, so transient conditions log as Primary provider unavailable (transient/rate-limit): ... while real credential failures keep the re-auth wording.

It also fixes the Fallback provider resolved: log line (issue #32790, surface #4): it printed runtime.get("provider"), the normalized internal runtime category, so an ollama fallback was mislabeled as openrouter even though traffic correctly hit the configured endpoint. It now reports the literal provider key from config (entry.get("provider")).

Scope note: the issue's most alarming surface (#1, the chat reply literally reading No Codex credentials stored) corresponds to a genuinely-missing-credentials AuthError (code=codex_auth_missing, relogin_required=True). The current credential-pool and token-refresh paths already classify a 429 distinctly from missing credentials (1-hour cooldown via EXHAUSTED_TTL_429_SECONDS, and codex tokens are only cleared on terminal refresh errors — not on a 429), so that exact string is not reproducible from a quota event against main. Confirming and fixing that surface needs the reporter's full traceback, so this PR uses Refs rather than Fixes.

Related Issue

Refs #32790

(Partial: addresses the gateway-log and fallback-resolution-label surfaces; the chat-reply root cause is deferred pending the reporter's traceback — see scope note above.)

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • gateway/run.py: in _resolve_runtime_agent_kwargs, branch the warning log on AuthError.relogin_required so transient (429/5xx) failures are not labeled "auth failed".
  • gateway/run.py: in _try_resolve_fallback_provider, log the configured entry.get("provider") on fallback resolution instead of the normalized runtime.get("provider") category.
  • tests/gateway/test_auth_fallback.py: add TestProviderErrorClassification covering (a) a transient 429 AuthError logging as transient, (b) a genuine credential AuthError still logging as auth-failed, (c) the fallback log naming ollama not openrouter.

How to Test

  1. pytest tests/gateway/test_auth_fallback.py -q — 6 passed (3 existing + 3 new).
  2. Manual: configure model.provider: openai-codex with a valid OAuth credential and an ollama fallback, then induce a token-refresh 429. The gateway log now reads Primary provider unavailable (transient/rate-limit): ... — trying fallback and Fallback provider resolved: ollama model=<X>.
  3. With a genuinely-missing credential (clear the codex auth state), the log still reads Primary provider auth failed: ..., preserving the re-auth guidance.

Platforms tested on: macOS (darwin-arm64), local — pytest tests/gateway/test_auth_fallback.py, plus tests/test_empty_model_fallback.py and tests/run_agent/test_provider_fallback.py (40 passed). Change is pure log-string/branching with no platform-specific APIs; Linux/Windows/WSL2 covered by CI.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run the relevant pytest suites and they pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS (darwin-arm64)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — log-string change only, no OS-specific APIs
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/gateway Gateway runner, session dispatch, delivery labels May 27, 2026
@konsisumer
konsisumer force-pushed the fix/gateway-classify-transient-provider-errors branch from 45d597f to 4542250 Compare May 30, 2026 14:49
@konsisumer

Copy link
Copy Markdown
Contributor Author

Rebased onto current origin/main to clear the merge conflict (DIRTY).

The conflict was confined to this PR's two files. Note for review: upstream landed an overlapping interim fix in _resolve_runtime_agent_kwargs that special-cased only the 429 case (is_rate_limited_auth_error). I resolved the conflict by keeping this PR's broader relogin_required classification — it covers transient 429 and 5xx (relogin_required=False) and supersedes the 429-only check — and removed the now-unused is_rate_limited_auth_error import. The fallback-label fix (surface #4) merged cleanly. Test conflict was import-only.

pytest tests/gateway/test_auth_fallback.py -q — 6 passed (3 existing + 3 new). ruff check clean on changed files.

@konsisumer

Copy link
Copy Markdown
Contributor Author

Closing — superseded by the consolidated #46643, which folds this PR's change in alongside the related work on the same issue. Reopen if that's not right.

@konsisumer konsisumer closed this Jun 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants