Skip to content

fix(provider): retry list_models on transient API failures - #90

Open
Sam Schillace (ramparte) wants to merge 1 commit into
microsoft:mainfrom
ramparte:fix/list-models-retry
Open

fix(provider): retry list_models on transient API failures#90
Sam Schillace (ramparte) wants to merge 1 commit into
microsoft:mainfrom
ramparte:fix/list-models-retry

Conversation

@ramparte

Copy link
Copy Markdown
Contributor

Problem

list_models() is the only network call in this module not protected by the module's own retry machinery. A single transient failure from /v1/models kills it outright.

The client is deliberately constructed with SDK retries off — AsyncAnthropic(..., max_retries=0), per the comment "We handle retries ourselves (SDK max_retries=0) to properly honor retry-after headers". self._retry_config exists and complete() uses it. list_models() did not:

response = await self.client.models.list()
api_models = list(response.data)

Why it matters. The routing-matrix resolver resolves glob model patterns by calling list_models() — the anthropic routing matrix maps every role through claude-opus-*, claude-sonnet-*, claude-haiku-*. When the call fails, resolve_model_role returns [] and never raises, so delegation silently falls back to the session's default model instead of the intended one. The user gets a different model with no error and no log line pointing at the cause.

This was observed in production on the OpenAI side twice within two hours — one HTTP 500 server_error, one Cloudflare 522 — each producing model_role 'fast' resolved to no candidates. Both were pure blips; a direct request immediately afterward returned 200 in ~0.5s. This module has the identical exposure against Anthropic's endpoint. Companion fix for the sibling provider: microsoft/amplifier-module-provider-openai#61.

What this does

Wraps the models.list() call in the module's existing retry_with_backoff with self._retry_config, mirroring how complete() wires it (guarded inner function + _on_retry hook). Uses the base self._retry_config rather than _build_retry_config(...), since that helper exists only to shrink the retry budget during model-fallback-on-overload, which does not apply to a parameterless GET.

Exception translation is copied from _do_complete, not reinvented:

  • AnthropicRateLimitError -> KernelRateLimitError (retryable, via the existing _parse_rate_limit_info)
  • AnthropicAuthenticationError -> KernelAuthenticationError (401, non-retryable)
  • AnthropicAPIStatusError -> 403 Cloudflare-challenge detection via the existing _is_cloudflare_challenge, else KernelAccessDeniedError; 404 -> KernelNotFoundError; >=500 -> KernelProviderUnavailableError (retryable); otherwise non-retryable KernelLLMError
  • Generic except Exception -> retryable KernelLLMError, matching this module's existing convention for connection/timeout errors (unlike the OpenAI sibling, this module does not explicitly import APIConnectionError/APITimeoutError and relies on this catch-all)

AnthropicOverloadedError (529) needs no separate branch — verified via __mro__ that it subclasses AnthropicAPIStatusError, so it is already caught and retried by the >=500 branch.

max_retries=0 on the AsyncAnthropic client is not changed — it is intentional so the module can honor retry-after headers itself.

All existing list_models() behavior is preserved exactly: the filtered parameter, _detect_family() grouping, and newest-per-family selection. Only the transport call is wrapped. The method still raises on persistent failure, so existing callers are unaffected — it just raises after exhausting retries. Docstring updated to say so.

Scope

Deliberately minimal. No caching added — the routing resolver already caches a successful result for the session's lifetime, so provider-level caching would be redundant and would widen the blast radius.

Testing

612 passing at base (68434cd) -> 616 after. No existing test modified. Four new tests in tests/test_list_models_retry.py:

  • succeeds first try — exactly 1 API call, asyncio.sleep never awaited, result unchanged (guards against behavior drift)
  • recovers from a transient 500 — more than one attempt, correct result
  • raises after retries exhausted — bounded attempt count, correct error type
  • non-retryable 401 raises immediately — 1 call, no sleep

ruff: 21 findings before and after, diff of the two lists is empty. pyright: 14 before and after, identical messages, line-shifted only.

list_models() was the only network call in this module not covered by
the shared retry_with_backoff()/_retry_config machinery. A single
transient failure (5xx, Cloudflare challenge, connection/timeout error)
killed it outright, causing the routing-matrix resolver's glob model
pattern resolution to silently degrade delegation to the session's
default model.

Wrap the client.models.list() call in retry_with_backoff(), mirroring
the exact error-translation and retry call shape already used by
complete() (see _do_complete/_on_retry at __init__.py:3150-3298 and
3458): 5xx and Cloudflare challenges retry, 401/403/404 raise
immediately as non-retryable kernel errors. Connection/timeout errors
fall through to the same generic catch-all _do_complete() uses.
AnthropicOverloadedError (529) is a subclass of AnthropicAPIStatusError
and is retried via the existing status >= 500 branch -- no new
classification introduced.

SDK-level retries remain disabled (max_retries=0 on the AsyncAnthropic
client) -- this module continues to own all retry policy via
retry-after headers.

Testing:
- New tests/test_list_models_retry.py: first-try success (single call,
  unchanged result), recovery after one transient 500, exhaustion after
  persistent 500s, and immediate raise on non-retryable 401.
- Full suite: 612 -> 616 passed (4 new tests), no regressions.
- ruff/pyright (via uvx): identical issue set before/after (21 ruff
  errors, 14 pyright errors -- all pre-existing, only line-shifted by
  the inserted code), zero new findings.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant