Support max_retries for non-OpenAI/Azure providers - #32896
Conversation
Previously max_retries was silently dropped for every provider except OpenAI/Azure (see the TODO in litellm/utils.py). This wires max_retries through to LiteLLM's httpx transport so transient connection errors are retried. - Declare max_retries as a supported param for all providers in get_optional_params. - AnthropicConfig maps max_retries and drops it from the request body. - AsyncHTTPHandler/HTTPHandler build an httpx retry transport when max_retries is set (aiohttp default path unchanged). - Anthropic handler creates a retry-configured client per request. - Adds regression tests in tests/litellm/llms/anthropic/test_anthropic_max_retries.py.
|
|
Greptile SummaryThis PR fixes a long-standing gap where
Confidence Score: 4/5Safe to merge for Anthropic users; callers on other providers accept the param without error but get no retries. The Anthropic wiring is thorough across all four call paths, body isolation in
|
| Filename | Overview |
|---|---|
| litellm/utils.py | Removes the legacy max_retries skip in _check_valid_arg and universally appends max_retries to supported_params; validation change is correct but creates a silent no-op for providers that have no retry wiring yet. |
| litellm/llms/anthropic/chat/transformation.py | Adds max_retries to supported params, maps it through map_openai_params, and correctly pops it in transform_request so it never reaches the Anthropic API body. |
| litellm/llms/anthropic/chat/handler.py | Wires max_retries to the httpx client in all four execution paths (async stream, async non-stream, sync stream, sync non-stream); logic is correct but the isinstance(optional_params, dict) guard is inconsistently applied. |
| litellm/llms/custom_httpx/http_handler.py | Adds max_retries parameter to AsyncHTTPHandler, HTTPHandler, and the transport factory methods; when set, the httpx retry transport is selected ahead of aiohttp, leaving the default path unchanged. |
| tests/litellm/llms/anthropic/test_anthropic_max_retries.py | New regression test file covering Anthropic mapping, body isolation, and transport configuration; test_default_handler_does_not_change_transport implicitly assumes aiohttp is installed and enabled, which makes it environment-dependent. |
Reviews (1): Last reviewed commit: "fix(anthropic): support max_retries for ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| assert not isinstance(handler.client._transport, httpx.AsyncHTTPTransport) | ||
|
|
||
|
|
||
| class TestMaxRetriesValidation: |
There was a problem hiding this comment.
This test will fail whenever aiohttp is not installed or when litellm.disable_aiohttp_transport = True / DISABLE_AIOHTTP_TRANSPORT=True is set. In those cases _create_async_transport falls through to _create_httpx_transport() (no args), which returns None, and httpx then internally materialises its own httpx.AsyncHTTPTransport as the default — so handler.client._transport is an httpx.AsyncHTTPTransport and the assertion fires a false failure. The test should either skip when aiohttp is unavailable or assert on the presence of the aiohttp transport rather than the absence of the httpx one.
| supported_params = supported_params or [] | ||
| allowed_openai_params = allowed_openai_params or [] | ||
| supported_params.extend(allowed_openai_params) | ||
| # ``max_retries`` is supported across all providers (OpenAI/Azure retry at | ||
| # the SDK layer; non-OpenAI providers retry at LiteLLM's httpx transport | ||
| # layer when ``max_retries`` is set). Declare it supported everywhere so it | ||
| # is no longer silently dropped for non-OpenAI providers. | ||
| if "max_retries" not in supported_params: | ||
| supported_params.append("max_retries") | ||
|
|
||
| _check_valid_arg( |
There was a problem hiding this comment.
max_retries accepted globally but only wired for Anthropic
Adding max_retries to supported_params for every provider means callers using Cohere, Bedrock, Vertex, Mistral, etc. will no longer get an UnsupportedParamsError, but the retries will silently not happen — those providers' map_openai_params implementations don't forward the param, so it never reaches the transport layer. A user who sets max_retries=3 expecting network-level retries on a transient Bedrock error will see the request fail without any retry attempt and no indication that the feature isn't active for that provider. Consider either raising a clear warning/log for providers that accept the param but don't wire it, or restricting the universal declaration to only providers whose handlers have actually been updated.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
ErenAta16
left a comment
ErenAta16
left a comment
There was a problem hiding this comment.
Anthropic's retry wiring itself (get_async_httpx_client(..., params={"max_retries": ...}) / _get_httpx_client(params={..., "max_retries": ...}), plus the new opt-in httpx-retry-transport path in custom_httpx/http_handler.py) is well-scoped and the transform_request/map_openai_params handling correctly keeps max_retries out of the actual Anthropic request body.
One scope concern though: the title and root-cause writeup describe this as fixing max_retries for "Anthropic, Cohere, Bedrock, Vertex, etc." generically, but the diff only wires the parameter through for Anthropic — utils.py's _check_valid_arg change declares max_retries a supported param for every provider (removing the old blanket skip), while the actual httpx-client wiring (the part that makes the parameter do anything) only exists in llms/anthropic/chat/handler.py. For Bedrock/Vertex/Cohere/etc., that means max_retries goes from "silently dropped, no warning" to "silently accepted as a supported param, still not actually wired to any retry behavior, still no warning" — which reads as a regression in discoverability for every non-Anthropic, non-OpenAI provider, even though nothing about their actual retry behavior changed. Worth either scoping the _check_valid_arg change to just Anthropic (and OpenAI/Azure, which already worked) until the other providers get the same handler-level wiring, or following up with the same pattern for Bedrock/Vertex/Cohere before this merges as currently titled.
345558e to
342a634
Compare
|
Thanks both for the thorough review — you both caught the same real issue from different angles. |
|
The updates address the key concerns well. A few observations on what's in the diff:
Beyond that, the approach looks correct. The Tip: You can customize Greptile's behavior for this repo with |
ErenAta16
left a comment
ErenAta16
left a comment
There was a problem hiding this comment.
Verified the two pieces I could check without the full build (Rust bridge still isn't buildable in my environment, same limitation as my earlier reviews on this repo, disclosing again here): map_openai_params's new branch is elif param == "max_retries" and isinstance(value, int), so a non-int value falls through and never gets set - matches test_map_openai_params_ignores_non_int_max_retries. And transform_request explicitly does optional_params.pop("max_retries", None) right where the other internal-only params (is_vertex_request, client_metadata) get stripped, so it can't leak into the actual Anthropic request body - matches the other test. Both check out against the diff, not just the test file's claims about itself.
+1 to Greptile's point on providers_with_retry_support living in two places (get_supported_openai_params.py and utils.py) with the identical literal {"openai", "azure", "anthropic"} - that's exactly the kind of duplication that silently drifts the next time someone wires up retries for another provider and only remembers to update one of the two call sites, which would put the two functions in disagreement about what's "supported" for that provider. Given both usages are already structurally identical (same set, same if provider in set and param not in supported: append shape), moving it to a single named constant in litellm/constants.py looks like a small, low-risk change worth doing in this PR rather than a follow-up, since it's touching both files anyway.
Scope, transport assertion, and the non-wired-provider regression test all look correct. Would like the constant extracted before merge, otherwise this is solid.
3 participants
Summary
litellm.completion(..., max_retries=N)was only honored for OpenAI and Azure. For every other provider (Anthropic, Cohere, Bedrock, Vertex, etc.) the parameter was silently dropped: no retries occurred and no error was raised. This PR wiresmax_retriesthrough to LiteLLM's httpx transport so transient connection errors are retried for non-OpenAI providers, and declares it a supported parameter everywhere.Root Cause
In
litellm/utils.pyget_optional_params, the_check_valid_arghelper contained a legacy TODO branch that explicitly skippedmax_retriesfor non-OpenAI providers. OpenAI/Azure consumemax_retriesvia their SDK client constructors (AsyncOpenAI(max_retries=...)), but the generic providers built their HTTP clients throughAsyncHTTPHandler/HTTPHandlerwith no retry transport configured. As a result the parameter never reached the transport layer for any provider other than OpenAI/Azure.Solution
litellm/utils.py: Removed the silent skip in_check_valid_arg.max_retriesis now appended to the supported-params list inget_optional_paramsso it is accepted for all providers (no moreUnsupportedParamsError, no silent drop).litellm/llms/anthropic/chat/transformation.py:AnthropicConfig.get_supported_openai_paramsnow listsmax_retries;map_openai_paramskeeps it inoptional_params;transform_requestpops it before building the JSON body so it is never sent to the Anthropic API.litellm/llms/custom_httpx/http_handler.py:AsyncHTTPHandler/HTTPHandleraccept amax_retriesargument. When set, the client is built with an httpx transport configured withretries=max_retries. The high-throughput aiohttp default path is unchanged — the httpx retry transport is only used whenmax_retriesis explicitly requested.litellm/llms/anthropic/chat/handler.py: The sync, async, and streaming paths now build a retry-configured httpx client (viaget_async_httpx_client/_get_httpx_clientwithmax_retries) when the request setsmax_retries.Alternatives considered: (1) mirroring the OpenAI SDK pattern per-provider is high-touch and provider-specific; (2) adding retries at the aiohttp transport layer would require new middleware. Routing through the existing httpx transport keeps the change small, centralized, and consistent with how
force_ipv4already selects the transport.Testing
tests/litellm/llms/anthropic/test_anthropic_max_retries.py:max_retriesis present inAnthropicConfig.get_supported_openai_params.map_openai_paramsretainsmax_retries(and ignores non-int values).transform_requestdropsmax_retriesfrom the request body while keeping real params.AsyncHTTPHandler(max_retries=N)/HTTPHandler(max_retries=N)build an httpx transport withretries == N.max_retries) transport path is unchanged.get_supported_openai_paramsincludesmax_retriesfor theanthropicprovider.get_optional_paramsacceptsmax_retriesforanthropicandcoherewithout error.AsyncHTTPHandlertests (tests/test_litellm/llms/custom_httpx/test_http_handler.py) and the Anthropic unit suite still pass (43 + 22 tests).Run with:
Performance Impact
None for the default path. The httpx retry transport is only instantiated when
max_retriesis explicitly set on a request.Backward Compatibility
max_retriescontinues to work for OpenAI/Azure as before; it now also works for other providers.max_retriesis never serialized into provider request bodies (explicitly popped intransform_request).Risk Assessment
optional_paramsthrough to the request body without mapping could now includemax_retries. Mitigation:map_openai_paramsfor each provider only copies recognized params intooptional_params, so unrecognizedmax_retriesis not forwarded; Anthropic additionally pops it.max_retriesswitch to the httpx transport for that client.max_retriesin their configs and passing it to the client factory.Files Changed
litellm/utils.pymax_retriessupported for all providers; remove silent skiplitellm/llms/anthropic/chat/transformation.pymax_retriesfor Anthropiclitellm/llms/anthropic/chat/handler.pylitellm/llms/custom_httpx/http_handler.pymax_retries-> httpx retry transporttests/litellm/llms/anthropic/test_anthropic_max_retries.pyReviewer Notes
_create_async_transport: whenmax_retries > 0we intentionally bypass the aiohttp default to use the httpx retry transport.transform_requestmust keep poppingmax_retriesso it never reaches the Anthropic API body.get_async_httpx_client/_get_httpx_clientincorporatemax_retries, so distinct retry counts get distinct (correctly configured) clients.Checklist
Related Issues
Fixes #32895