From 02f63d20bb563f7ebbd5396d4be85db66a4f26e4 Mon Sep 17 00:00:00 2001 From: milan-berri Date: Tue, 23 Jun 2026 19:16:15 +0200 Subject: [PATCH] fix(router): guard num_retries=None in async_function_with_retries (#30036) When num_retries reaches async_function_with_retries as None - e.g. a caller passes num_retries=None explicitly (dict.get() does not fall back on an existing None value), an auto_router/complexity_router path does not propagate it, or Router.update_settings(num_retries=None) is used - the comparison `if num_retries > 0:` raised: TypeError: '>' not supported between instances of 'NoneType' and 'int' This only surfaced when the underlying call failed with a retryable error (rate limit / connection / 5xx), so the real upstream error was masked by a confusing TypeError. Normalise an explicit num_retries=None to the router default in _update_kwargs_before_fallbacks (falling back to 0 when the router default is itself None, and preserving an explicit 0), and keep the matching guard at the single pop site in async_function_with_retries as the safety net for paths that bypass the setter. Adds regression tests to test_router_per_deployment_num_retries.py. Relates to #23316, #25889, #23699, #28126 Co-authored-by: Cursor Co-authored-by: yuneng-jiang --- litellm/router.py | 19 ++- .../test_router_per_deployment_num_retries.py | 133 +++++++++++++++++- 2 files changed, 149 insertions(+), 3 deletions(-) diff --git a/litellm/router.py b/litellm/router.py index acce1c58a8b..c687b2d9f67 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -3134,7 +3134,18 @@ class Router: - litellm_trace_id - metadata """ - kwargs["num_retries"] = kwargs.get("num_retries", self.num_retries) + # Normalise an explicit num_retries=None to the router default here (dict.get() + # only falls back when the key is absent, not when its value is None), then to 0 + # if the router default is itself None - mirroring the guard in + # async_function_with_retries, which remains the safety net for paths that bypass + # this setter. + _req_num_retries = kwargs.get("num_retries") + if _req_num_retries is not None: + kwargs["num_retries"] = _req_num_retries + else: + kwargs["num_retries"] = ( + self.num_retries if self.num_retries is not None else 0 + ) kwargs.setdefault("litellm_trace_id", str(uuid.uuid4())) model_group_alias: Optional[str] = None if self._get_model_from_alias(model=model): @@ -6931,7 +6942,11 @@ class Router: "model_group_retry_policy", self.model_group_retry_policy ) model_group: Optional[str] = kwargs.get("model") - num_retries = kwargs.pop("num_retries") + num_retries = kwargs.pop("num_retries", None) + if num_retries is None: + # Fall back to the router setting (then 0) so the comparisons below never + # hit `None > int`, which would mask the real upstream error with a TypeError. + num_retries = self.num_retries if self.num_retries is not None else 0 ## ADD MODEL GROUP SIZE TO METADATA - used for model_group_rate_limit_error tracking _metadata: dict = kwargs.get("litellm_metadata", kwargs.get("metadata")) or {} 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 154ba579e4e..af2372616a6 100644 --- a/tests/test_litellm/test_router_per_deployment_num_retries.py +++ b/tests/test_litellm/test_router_per_deployment_num_retries.py @@ -4,8 +4,9 @@ GitHub Issue: #18968 - Per-deployment max_retries/num_retries in litellm_params """ import pytest -from unittest.mock import MagicMock, patch +from unittest.mock import patch +import litellm from litellm import Router @@ -188,3 +189,133 @@ class TestPerDeploymentNumRetries: # Verify num_retries was converted from string to int assert exc.num_retries == 6 + + +class TestNumRetriesNoneGuard: + """ + Regression tests for the num_retries=None TypeError in async_function_with_retries. + + When num_retries reaches async_function_with_retries as None - e.g. a caller passes + num_retries=None explicitly (dict.get() does not fall back on an existing None value), + an auto_router/complexity_router path does not propagate it, or + Router.update_settings(num_retries=None) is used - AND the underlying call fails with a + retryable error, the comparison `if num_retries > 0:` raised: + + TypeError: '>' not supported between instances of 'NoneType' and 'int' + + This masked the real upstream error (rate limit / connection / 5xx) behind a TypeError. + Related issues: #23316, #25889, #23699, #28126. + """ + + @staticmethod + def _mock_router(num_retries=2): + return Router( + model_list=[ + { + "model_name": "mock-model", + "litellm_params": { + "model": "gpt-4o-mini", + "mock_response": "ok", + }, + } + ], + num_retries=num_retries, + ) + + def test_update_kwargs_normalises_explicit_none_to_router_default(self): + """ + _update_kwargs_before_fallbacks must normalise an explicit num_retries=None to + the router default (not leave it as None), while preserving an explicit 0. + """ + router = self._mock_router(num_retries=4) + + # explicit None -> router default + kwargs = {"num_retries": None} + router._update_kwargs_before_fallbacks(model="mock-model", kwargs=kwargs) + assert kwargs["num_retries"] == 4 + + # explicit 0 is preserved (retries stay disabled) + kwargs = {"num_retries": 0} + router._update_kwargs_before_fallbacks(model="mock-model", kwargs=kwargs) + assert kwargs["num_retries"] == 0 + + # absent -> router default (unchanged behaviour) + kwargs = {} + router._update_kwargs_before_fallbacks(model="mock-model", kwargs=kwargs) + assert kwargs["num_retries"] == 4 + + # explicit None with router default also None -> 0 (mirrors the downstream guard) + router.num_retries = None # simulate update_settings(num_retries=None) (#28126) + kwargs = {"num_retries": None} + router._update_kwargs_before_fallbacks(model="mock-model", kwargs=kwargs) + assert kwargs["num_retries"] == 0 + + @pytest.mark.asyncio + async def test_acompletion_num_retries_none_does_not_raise_typeerror(self): + """ + Per-request num_retries=None + a retryable error must NOT raise TypeError. + The router falls back to its configured num_retries and retries the (transient) + error, so the request succeeds. + """ + router = self._mock_router(num_retries=2) + with patch("asyncio.sleep", return_value=None): + response = await router.acompletion( + model="mock-model", + messages=[{"role": "user", "content": "hi"}], + num_retries=None, # the trigger + mock_testing_rate_limit_error=True, # retryable error path + ) + assert response.choices[0].message.content == "ok" + + @pytest.mark.asyncio + async def test_async_function_with_retries_none_falls_back_to_zero(self): + """ + When both the per-request value AND the router-level setting are None + (e.g. after Router.update_settings(num_retries=None), #28126), num_retries must + fall back to 0 and the real retryable error must surface - not a TypeError. + """ + router = self._mock_router(num_retries=0) + router.num_retries = None # simulate update_settings(num_retries=None) + + async def failing_fn(*args, **kwargs): + raise litellm.RateLimitError( + message="boom", model="mock-model", llm_provider="openai" + ) + + with patch("asyncio.sleep", return_value=None): + with pytest.raises(litellm.RateLimitError): + await router.async_function_with_retries( + original_function=failing_fn, + model="mock-model", + messages=[{"role": "user", "content": "hi"}], + num_retries=None, + ) + + @pytest.mark.asyncio + async def test_async_function_with_retries_none_falls_back_to_router_default(self): + """ + A None per-request num_retries falls back to the router-level setting, so retries + still happen (original_function is invoked more than once) before the real error + is raised - proving None did not silently disable retries or crash. + """ + router = self._mock_router(num_retries=3) + calls = {"n": 0} + + async def failing_fn(*args, **kwargs): + calls["n"] += 1 + raise litellm.InternalServerError( + message="boom", model="mock-model", llm_provider="openai" + ) + + with patch("asyncio.sleep", return_value=None): + with pytest.raises(litellm.InternalServerError): + await router.async_function_with_retries( + original_function=failing_fn, + model="mock-model", + messages=[{"role": "user", "content": "hi"}], + metadata={}, # populated by acompletion in the real path; log_retry needs it + num_retries=None, + ) + + # 1 initial attempt + at least 1 retry -> proves None fell back to a positive int + assert calls["n"] >= 2