mirror of
https://github.com/tiennm99/litellm.git
synced 2026-08-20 08:23:47 +00:00
fix(router): prevent retrying 4xx client errors (#19275)
This commit is contained in:
@@ -4865,6 +4865,12 @@ class Router:
|
||||
):
|
||||
raise error
|
||||
|
||||
status_code = getattr(error, "status_code", None)
|
||||
if status_code is not None and not litellm._should_retry(status_code):
|
||||
# 401/403 are special cases - allow retry if multiple deployments exist (handled below)
|
||||
if status_code not in (401, 403):
|
||||
raise error
|
||||
|
||||
if isinstance(error, litellm.NotFoundError):
|
||||
raise error
|
||||
# Error we should only retry if there are other deployments
|
||||
|
||||
@@ -676,6 +676,85 @@ def test_no_retry_for_not_found_error_404():
|
||||
print("got exception", e)
|
||||
|
||||
|
||||
def test_no_retry_for_bad_request_error_400():
|
||||
"""
|
||||
Test that 400 BadRequestError is NOT retried, even if healthy deployments exist.
|
||||
This tests the fix for GitHub issue #19216.
|
||||
"""
|
||||
healthy_deployments = ["deployment1", "deployment2"] # Multiple healthy deployments
|
||||
|
||||
router = Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "gpt-3.5-turbo",
|
||||
"litellm_params": {
|
||||
"model": "azure/gpt-4.1-mini",
|
||||
"api_key": os.getenv("AZURE_API_KEY"),
|
||||
"api_version": os.getenv("AZURE_API_VERSION"),
|
||||
"api_base": os.getenv("AZURE_API_BASE"),
|
||||
},
|
||||
}
|
||||
]
|
||||
)
|
||||
|
||||
# Act & Assert
|
||||
error = litellm.BadRequestError(
|
||||
message="400 Invalid request parameters",
|
||||
model="gpt-3.5-turbo",
|
||||
llm_provider="azure",
|
||||
)
|
||||
try:
|
||||
response = router.should_retry_this_error(
|
||||
error=error, healthy_deployments=healthy_deployments
|
||||
)
|
||||
pytest.fail(
|
||||
"Should have raised BadRequestError - 400 errors should never be retried"
|
||||
)
|
||||
except litellm.BadRequestError as e:
|
||||
print("Correctly raised BadRequestError without retry:", e)
|
||||
|
||||
|
||||
def test_no_retry_for_unprocessable_entity_error_422():
|
||||
"""
|
||||
Test that 422 UnprocessableEntityError is NOT retried, even if healthy deployments exist.
|
||||
"""
|
||||
healthy_deployments = ["deployment1", "deployment2"] # Multiple healthy deployments
|
||||
|
||||
router = Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "gpt-3.5-turbo",
|
||||
"litellm_params": {
|
||||
"model": "azure/gpt-4.1-mini",
|
||||
"api_key": os.getenv("AZURE_API_KEY"),
|
||||
"api_version": os.getenv("AZURE_API_VERSION"),
|
||||
"api_base": os.getenv("AZURE_API_BASE"),
|
||||
},
|
||||
}
|
||||
]
|
||||
)
|
||||
|
||||
# Act & Assert
|
||||
error = litellm.UnprocessableEntityError(
|
||||
message="422 Unprocessable Entity",
|
||||
model="gpt-3.5-turbo",
|
||||
llm_provider="azure",
|
||||
response=httpx.Response(
|
||||
status_code=422,
|
||||
request=httpx.Request(method="POST", url="https://api.openai.com/v1"),
|
||||
),
|
||||
)
|
||||
try:
|
||||
response = router.should_retry_this_error(
|
||||
error=error, healthy_deployments=healthy_deployments
|
||||
)
|
||||
pytest.fail(
|
||||
"Should have raised UnprocessableEntityError - 422 errors should never be retried"
|
||||
)
|
||||
except litellm.UnprocessableEntityError as e:
|
||||
print("Correctly raised UnprocessableEntityError without retry:", e)
|
||||
|
||||
|
||||
internal_server_error = litellm.InternalServerError(
|
||||
message="internal server error",
|
||||
model="gpt-12",
|
||||
@@ -809,7 +888,7 @@ async def test_router_timeout_model_specific_and_global():
|
||||
async def test_router_retry_num_retries_tracking():
|
||||
"""
|
||||
Test that num_retries attribute is correctly set on exceptions when all retries are exhausted.
|
||||
|
||||
|
||||
This verifies the fix for the bug where num_retries was incorrectly set to current_attempt
|
||||
(0-indexed) instead of the actual number of retries attempted.
|
||||
"""
|
||||
@@ -837,8 +916,17 @@ async def test_router_retry_num_retries_tracking():
|
||||
)
|
||||
|
||||
with patch.object(router, "make_call", side_effect=mock_make_call):
|
||||
with patch.object(router, "_async_get_healthy_deployments", return_value=([{"model_info": {"id": "test-id"}}], [{"model_info": {"id": "test-id"}}])):
|
||||
with patch.object(router, "_time_to_sleep_before_retry", return_value=0.01): # Fast retries for testing
|
||||
with patch.object(
|
||||
router,
|
||||
"_async_get_healthy_deployments",
|
||||
return_value=(
|
||||
[{"model_info": {"id": "test-id"}}],
|
||||
[{"model_info": {"id": "test-id"}}],
|
||||
),
|
||||
):
|
||||
with patch.object(
|
||||
router, "_time_to_sleep_before_retry", return_value=0.01
|
||||
): # Fast retries for testing
|
||||
try:
|
||||
await router.acompletion(
|
||||
model="gpt-3.5-turbo",
|
||||
@@ -847,15 +935,27 @@ async def test_router_retry_num_retries_tracking():
|
||||
pytest.fail("Expected exception to be raised")
|
||||
except litellm.RateLimitError as e:
|
||||
# Verify num_retries is correctly set to 3 (not 2, which would be current_attempt)
|
||||
assert hasattr(e, "num_retries"), "Exception should have num_retries attribute"
|
||||
assert hasattr(e, "max_retries"), "Exception should have max_retries attribute"
|
||||
assert e.num_retries == 3, f"Expected num_retries to be 3, got {e.num_retries}"
|
||||
assert e.max_retries == 3, f"Expected max_retries to be 3, got {e.max_retries}"
|
||||
|
||||
assert hasattr(
|
||||
e, "num_retries"
|
||||
), "Exception should have num_retries attribute"
|
||||
assert hasattr(
|
||||
e, "max_retries"
|
||||
), "Exception should have max_retries attribute"
|
||||
assert (
|
||||
e.num_retries == 3
|
||||
), f"Expected num_retries to be 3, got {e.num_retries}"
|
||||
assert (
|
||||
e.max_retries == 3
|
||||
), f"Expected max_retries to be 3, got {e.max_retries}"
|
||||
|
||||
# Verify the error message includes correct retry information
|
||||
error_str = str(e)
|
||||
assert "LiteLLM Retried: 3 times" in error_str, f"Error message should indicate 3 retries: {error_str}"
|
||||
assert "LiteLLM Max Retries: 3" in error_str, f"Error message should show max retries: {error_str}"
|
||||
assert (
|
||||
"LiteLLM Retried: 3 times" in error_str
|
||||
), f"Error message should indicate 3 retries: {error_str}"
|
||||
assert (
|
||||
"LiteLLM Max Retries: 3" in error_str
|
||||
), f"Error message should show max retries: {error_str}"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -887,7 +987,14 @@ async def test_router_retry_num_retries_single_retry():
|
||||
)
|
||||
|
||||
with patch.object(router, "make_call", side_effect=mock_make_call):
|
||||
with patch.object(router, "_async_get_healthy_deployments", return_value=([{"model_info": {"id": "test-id"}}], [{"model_info": {"id": "test-id"}}])):
|
||||
with patch.object(
|
||||
router,
|
||||
"_async_get_healthy_deployments",
|
||||
return_value=(
|
||||
[{"model_info": {"id": "test-id"}}],
|
||||
[{"model_info": {"id": "test-id"}}],
|
||||
),
|
||||
):
|
||||
with patch.object(router, "_time_to_sleep_before_retry", return_value=0.01):
|
||||
try:
|
||||
await router.acompletion(
|
||||
@@ -897,5 +1004,9 @@ async def test_router_retry_num_retries_single_retry():
|
||||
pytest.fail("Expected exception to be raised")
|
||||
except litellm.Timeout as e:
|
||||
# With num_retries=1, we should attempt 1 retry
|
||||
assert e.num_retries == 1, f"Expected num_retries to be 1, got {e.num_retries}"
|
||||
assert e.max_retries == 1, f"Expected max_retries to be 1, got {e.max_retries}"
|
||||
assert (
|
||||
e.num_retries == 1
|
||||
), f"Expected num_retries to be 1, got {e.num_retries}"
|
||||
assert (
|
||||
e.max_retries == 1
|
||||
), f"Expected max_retries to be 1, got {e.max_retries}"
|
||||
|
||||
Reference in New Issue
Block a user