From f4d60ab9d974475ac19c132637d4954d33d4c94c Mon Sep 17 00:00:00 2001 From: Yassin Kortam Date: Tue, 21 Jul 2026 11:37:48 -0700 Subject: [PATCH 1/2] fix(router): stop per-deployment num_retries from double-counting as provider max_retries A model group with one deployment and num_retries set in the deployment's litellm_params sent (1 + num_retries) ** 2 requests upstream instead of 1 + num_retries. The deployment's num_retries reached litellm.completion, which copied it onto max_retries and set it on the provider client, so the provider SDK retried num_retries times inside each of the Router's 1 + num_retries attempts. The Router is the sole retry owner for routed calls, so completion() now forces the provider-SDK max_retries to 0 whenever the call originates from the Router/proxy (detected via model_group in the request metadata) and only keeps the num_retries to max_retries alias for direct, non-routed litellm calls (the instructor use case). This also stops a request- or deployment-level max_retries from nesting on top of the Router's retries. Resolves LIT-4385 --- litellm/main.py | 5 +- .../test_router_per_deployment_num_retries.py | 141 ++++++++++++++++++ 2 files changed, 145 insertions(+), 1 deletion(-) diff --git a/litellm/main.py b/litellm/main.py index fb05a375111a..7d4205e6aef8 100644 --- a/litellm/main.py +++ b/litellm/main.py @@ -5111,7 +5111,10 @@ def completion( # type: ignore try: if base_url is not None: api_base = base_url - if num_retries is not None: + is_router_call = "model_group" in (kwargs.get("metadata") or kwargs.get("litellm_metadata") or {}) + if is_router_call: + max_retries = 0 + elif num_retries is not None: max_retries = num_retries logging: LiteLLMLoggingObj = cast(LiteLLMLoggingObj, litellm_logging_obj) fallbacks = fallbacks or litellm.model_fallbacks diff --git a/tests/test_litellm/test_router_per_deployment_num_retries.py b/tests/test_litellm/test_router_per_deployment_num_retries.py index af2372616a68..53c01e18f65c 100644 --- a/tests/test_litellm/test_router_per_deployment_num_retries.py +++ b/tests/test_litellm/test_router_per_deployment_num_retries.py @@ -3,11 +3,13 @@ GitHub Issue: #18968 - Per-deployment max_retries/num_retries in litellm_params is not used in retry logic """ +import httpx import pytest from unittest.mock import patch import litellm from litellm import Router +from litellm.types.router import RetryPolicy class TestPerDeploymentNumRetries: @@ -319,3 +321,142 @@ async def failing_fn(*args, **kwargs): # 1 initial attempt + at least 1 retry -> proves None fell back to a positive int assert calls["n"] >= 2 + + +class TestNoProviderRetryAmplification: + """ + A routed request must reach the upstream provider exactly ``1 + `` + times. The Router is the sole retry owner for routed calls, so the provider SDK + must never retry on top of it. Otherwise a per-deployment ``num_retries`` set in + ``litellm_params`` is applied twice - once by the Router loop and once as the + provider client's ``max_retries`` - turning one request into ``(1 + num_retries) ** 2`` + upstream requests. + + These tests count actual upstream HTTP requests through the full Router completion + path by injecting a counting transport via ``litellm.aclient_session`` (the + documented seam the OpenAI client builder reads), so both Router-level and any + provider-SDK-level retries are observed. + """ + + @staticmethod + def _install_counting_upstream() -> dict: + """Route every upstream POST to a 500 and count it. ``retry-after: 0`` keeps + provider-SDK backoff at zero so a mutated (double-retrying) build stays fast.""" + counter = {"n": 0} + + def handler(request: httpx.Request) -> httpx.Response: + counter["n"] += 1 + return httpx.Response( + 500, + headers={"retry-after": "0"}, + json={"error": {"message": "boom", "type": "server_error"}}, + ) + + litellm.aclient_session = httpx.AsyncClient(transport=httpx.MockTransport(handler)) + return counter + + @pytest.fixture(autouse=True) + def _isolate_clients(self): + litellm.in_memory_llm_clients_cache.flush_cache() + yield + litellm.aclient_session = None + litellm.in_memory_llm_clients_cache.flush_cache() + + @staticmethod + def _router(api_base: str, litellm_params: dict, **router_kwargs) -> Router: + params = {"model": "openai/gpt-4o-mini", "api_base": api_base, "api_key": "sk-fake"} + params.update(litellm_params) + return Router(model_list=[{"model_name": "mock", "litellm_params": params}], **router_kwargs) + + async def _call_and_count(self, router: Router, **call_kwargs) -> int: + counter = self._install_counting_upstream() + with patch("asyncio.sleep", return_value=None): + with pytest.raises(litellm.InternalServerError): + await router.acompletion( + model="mock", messages=[{"role": "user", "content": "hi"}], **call_kwargs + ) + return counter["n"] + + @pytest.mark.asyncio + @pytest.mark.parametrize("num_retries", [2, 5]) + async def test_deployment_num_retries_sends_no_extra_provider_requests(self, num_retries): + """ + Deployment ``num_retries=N`` (every attempt failing) must send exactly ``N + 1`` + upstream requests, not ``(N + 1) ** 2``. This is the amplification regression: + an unfixed build sends 9 (N=2) or 36 (N=5). + """ + counter = self._install_counting_upstream() + router = self._router( + f"https://amp-{num_retries}.local/v1", {"num_retries": num_retries}, num_retries=1 + ) + with patch("asyncio.sleep", return_value=None): + with pytest.raises(litellm.InternalServerError): + await router.acompletion(model="mock", messages=[{"role": "user", "content": "hi"}]) + assert counter["n"] == num_retries + 1 + + @pytest.mark.asyncio + async def test_request_max_retries_does_not_nest_with_router_retries(self): + """ + A request-body ``max_retries`` must not make the provider SDK retry on top of the + Router. With deployment ``num_retries=5`` and request ``max_retries=3`` the count + stays ``6``; a build that lets either value reach the provider SDK sends 24 or 36. + """ + router = self._router("https://nest-req.local/v1", {"num_retries": 5}, num_retries=1) + assert await self._call_and_count(router, max_retries=3) == 6 + + @pytest.mark.asyncio + async def test_deployment_max_retries_does_not_nest_with_router_retries(self): + """ + A deployment-level ``max_retries`` is likewise never applied on top of the Router's + retries for a routed call: deployment ``num_retries=5`` plus ``max_retries=3`` still + sends exactly ``6`` upstream requests. + """ + router = self._router( + "https://nest-dep.local/v1", {"num_retries": 5, "max_retries": 3}, num_retries=1 + ) + assert await self._call_and_count(router) == 6 + + @pytest.mark.asyncio + async def test_retry_policy_configured_does_not_reintroduce_amplification(self): + """ + With a retry policy configured alongside a per-deployment ``num_retries=5``, the + provider SDK still must not retry: exactly ``6`` upstream requests, not 36. + """ + router = self._router( + "https://policy.local/v1", + {"num_retries": 5}, + num_retries=1, + retry_policy=RetryPolicy(InternalServerErrorRetries=2), + ) + assert await self._call_and_count(router) == 6 + + @pytest.mark.asyncio + async def test_global_num_retries_not_amplified(self): + """ + Global ``num_retries`` (no per-deployment setting) already behaves correctly and + must stay that way: ``num_retries=3`` sends ``4`` upstream requests. + """ + router = self._router("https://global.local/v1", {}, num_retries=3) + assert await self._call_and_count(router) == 4 + + @pytest.mark.asyncio + async def test_direct_completion_still_forwards_num_retries_to_provider(self): + """ + For a NON-routed direct ``litellm.acompletion`` call, ``num_retries`` remains an + alias for the provider client's ``max_retries`` (the instructor use case). The + provider SDK therefore retries in addition to litellm's own retry wrapper, so the + upstream count exceeds ``num_retries + 1`` - proving the routed-call fix did not + change direct-call behaviour. + """ + counter = self._install_counting_upstream() + num_retries = 2 + with patch("asyncio.sleep", return_value=None): + with pytest.raises(litellm.InternalServerError): + await litellm.acompletion( + model="openai/gpt-4o-mini", + api_base="https://direct.local/v1", + api_key="sk-fake", + messages=[{"role": "user", "content": "hi"}], + num_retries=num_retries, + ) + assert counter["n"] > num_retries + 1 From 688b06443dc8f5f888622a5153e5d1906d1acb0e Mon Sep 17 00:00:00 2001 From: Yassin Kortam Date: Tue, 21 Jul 2026 11:43:49 -0700 Subject: [PATCH 2/2] fix(router): make router-origin check robust and close test clients Address review: detect the router marker in both metadata and litellm_metadata independently (a non-empty metadata without model_group no longer hides a model_group in litellm_metadata), and close the injected async clients in the test fixture. --- litellm/main.py | 2 +- .../test_router_per_deployment_num_retries.py | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/litellm/main.py b/litellm/main.py index 7d4205e6aef8..dc3ec469a1b5 100644 --- a/litellm/main.py +++ b/litellm/main.py @@ -5111,7 +5111,7 @@ def completion( # type: ignore try: if base_url is not None: api_base = base_url - is_router_call = "model_group" in (kwargs.get("metadata") or kwargs.get("litellm_metadata") or {}) + is_router_call = any("model_group" in (kwargs.get(k) or ()) for k in ("metadata", "litellm_metadata")) if is_router_call: max_retries = 0 elif num_retries is not None: diff --git a/tests/test_litellm/test_router_per_deployment_num_retries.py b/tests/test_litellm/test_router_per_deployment_num_retries.py index 53c01e18f65c..39927eb1d5b8 100644 --- a/tests/test_litellm/test_router_per_deployment_num_retries.py +++ b/tests/test_litellm/test_router_per_deployment_num_retries.py @@ -5,6 +5,7 @@ import httpx import pytest +import pytest_asyncio from unittest.mock import patch import litellm @@ -355,12 +356,15 @@ def handler(request: httpx.Request) -> httpx.Response: litellm.aclient_session = httpx.AsyncClient(transport=httpx.MockTransport(handler)) return counter - @pytest.fixture(autouse=True) - def _isolate_clients(self): + @pytest_asyncio.fixture(autouse=True) + async def _isolate_clients(self): litellm.in_memory_llm_clients_cache.flush_cache() yield + session = litellm.aclient_session litellm.aclient_session = None litellm.in_memory_llm_clients_cache.flush_cache() + if session is not None: + await session.aclose() @staticmethod def _router(api_base: str, litellm_params: dict, **router_kwargs) -> Router: