From 65e943dc2b99b68e5d2b0a23653c8670c282fa48 Mon Sep 17 00:00:00 2001 From: moh-dev-stack Date: Fri, 23 Jan 2026 03:39:58 +0000 Subject: [PATCH] Bugfix/19481 num retries env var type (#19507) * Enhance error handling for num_retries in Router class to support string values. Add test case to verify conversion from string to int for deployment num_retries. * Refactor Router class for improved readability by formatting long lines and enhancing exception handling tests for num_retries. Ensure consistent style in test cases for better maintainability. * Update exception handling for num_retries in Router class to suppress mypy warnings. Add type ignore comment for clarity in type conversion from string to int. --- litellm/router.py | 19 +++++-- .../test_router_per_deployment_num_retries.py | 57 +++++++++++++++---- 2 files changed, 59 insertions(+), 17 deletions(-) diff --git a/litellm/router.py b/litellm/router.py index 62070d2375..3234366cfb 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -1696,8 +1696,11 @@ class Router: litellm_params = deployment.get("litellm_params", {}) dep_num_retries = litellm_params.get("num_retries") - if dep_num_retries is not None and isinstance(dep_num_retries, int): - exception.num_retries = dep_num_retries # type: ignore + if dep_num_retries is not None: + try: + exception.num_retries = int(dep_num_retries) # type: ignore # Handle both int and str + except (ValueError, TypeError): + pass # Skip if value can't be converted to int def _update_kwargs_with_default_litellm_params( self, kwargs: dict, metadata_variable_name: Optional[str] = "metadata" @@ -4692,9 +4695,12 @@ class Router: # get num_retries from retry policy # Use the model_group captured at the start of the function, or get it from metadata # kwargs.get("model") at this point is the deployment model, not the model_group - _model_group_for_retry_policy = model_group or _metadata.get("model_group") or kwargs.get("model") + _model_group_for_retry_policy = ( + model_group or _metadata.get("model_group") or kwargs.get("model") + ) _retry_policy_retries = self.get_num_retries_from_retry_policy( - exception=original_exception, model_group=_model_group_for_retry_policy + exception=original_exception, + model_group=_model_group_for_retry_policy, ) if _retry_policy_retries is not None: num_retries = _retry_policy_retries @@ -5879,7 +5885,10 @@ class Router: ) # done reading model["litellm_params"] # Check if provider is supported: either in enum or JSON-configured - if custom_llm_provider not in litellm.provider_list and not JSONProviderRegistry.exists(custom_llm_provider): + if ( + custom_llm_provider not in litellm.provider_list + and not JSONProviderRegistry.exists(custom_llm_provider) + ): raise Exception(f"Unsupported provider - {custom_llm_provider}") #### DEPLOYMENT NAMES INIT ######## 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 4021ca2807..154ba579e4 100644 --- a/tests/test_litellm/test_router_per_deployment_num_retries.py +++ b/tests/test_litellm/test_router_per_deployment_num_retries.py @@ -32,17 +32,17 @@ class TestPerDeploymentNumRetries: ) deployment = router.model_list[0] - + # Create a mock exception without num_retries class MockException(Exception): pass - + exc = MockException("test error") assert not hasattr(exc, "num_retries") or exc.num_retries is None - + # Call the helper router._set_deployment_num_retries_on_exception(exc, deployment) - + # Verify num_retries was set from deployment assert exc.num_retries == 5 @@ -66,16 +66,16 @@ class TestPerDeploymentNumRetries: ) deployment = router.model_list[0] - + # Create an exception that already has num_retries class MockException(Exception): num_retries = 10 # Already set - + exc = MockException("test error") - + # Call the helper router._set_deployment_num_retries_on_exception(exc, deployment) - + # Verify num_retries was NOT overridden assert exc.num_retries == 10 @@ -99,15 +99,15 @@ class TestPerDeploymentNumRetries: ) deployment = router.model_list[0] - + class MockException(Exception): pass - + exc = MockException("test error") - + # Call the helper router._set_deployment_num_retries_on_exception(exc, deployment) - + # Verify num_retries was not set (deployment has no num_retries) assert not hasattr(exc, "num_retries") or exc.num_retries is None @@ -155,3 +155,36 @@ class TestPerDeploymentNumRetries: kwargs = {} router._update_kwargs_before_fallbacks(model="test-model", kwargs=kwargs) assert kwargs["num_retries"] == 7 # Uses global + + def test_set_deployment_num_retries_with_string_value(self): + """ + Test that _set_deployment_num_retries_on_exception handles string values + from environment variables correctly. + GitHub Issue: #19481 + """ + router = Router( + model_list=[ + { + "model_name": "test-model", + "litellm_params": { + "model": "openai/gpt-4", + "api_key": "test-key", + "num_retries": "6", # String value (as from env var) + }, + }, + ], + num_retries=0, # Global setting + ) + + deployment = router.model_list[0] + + class MockException(Exception): + pass + + exc = MockException("test error") + + # Call the helper + router._set_deployment_num_retries_on_exception(exc, deployment) + + # Verify num_retries was converted from string to int + assert exc.num_retries == 6