fix(parallel-request-limiter): drop None from detail; annotate raise_rate_limit_error as NoReturn

The v1 ' raise_rate_limit_error' helper built an unused 'error_message'
variable and then assembled the actual ' detail' via an f-string that
interpolated 'additional_details' verbatim — producing
'Max parallel request limit reached None' when invoked without
arguments (flagged by code review).

Fix the helper to:
- use the constructed 'error_message' as the detail
- annotate the helper as NoReturn since it always raises
- drop the redundant 'raise'/'return' at the two call sites

Add two regression tests covering both the with- and without-
additional_details paths.

LIT-2968

Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
This commit is contained in:
Cursor Agent 2026-05-11 23:19:15 +00:00
parent d0202b802e
commit 4e5abe1710
No known key found for this signature in database
2 changed files with 26 additions and 5 deletions

View file

@ -1,7 +1,7 @@
import asyncio
import sys
from datetime import datetime, timedelta
from typing import TYPE_CHECKING, Any, List, Literal, Optional, Tuple, Union
from typing import TYPE_CHECKING, Any, List, Literal, NoReturn, Optional, Tuple, Union
from pydantic import BaseModel
from typing_extensions import TypedDict
@ -72,7 +72,7 @@ class _PROXY_MaxParallelRequestsHandler(CustomLogger):
if current is None:
if max_parallel_requests == 0 or tpm_limit == 0 or rpm_limit == 0:
# base case
raise self.raise_rate_limit_error(
self.raise_rate_limit_error(
additional_details=f"{CommonProxyErrors.max_parallel_request_limit_reached.value}. Hit limit for {rate_limit_type}. Current limits: max_parallel_requests: {max_parallel_requests}, tpm_limit: {tpm_limit}, rpm_limit: {rpm_limit}"
)
new_val = {
@ -122,11 +122,11 @@ class _PROXY_MaxParallelRequestsHandler(CustomLogger):
def raise_rate_limit_error(
self, additional_details: Optional[str] = None
) -> ProxyRateLimitError:
) -> NoReturn:
"""
Raise a 429 with a retry-after header for litellm-proxy parallel-request limits.
Returns a :class:`ProxyRateLimitError`, which is both a
Raises a :class:`ProxyRateLimitError`, which is both a
:class:`litellm.RateLimitError` (so callers can catch by category) and a
:class:`fastapi.HTTPException` (so the FastAPI dispatcher serializes it
correctly with status 429 and the supplied headers).
@ -227,7 +227,7 @@ class _PROXY_MaxParallelRequestsHandler(CustomLogger):
current_global_requests = 1
# if above -> raise error
if current_global_requests >= global_max_parallel_requests:
return self.raise_rate_limit_error(
self.raise_rate_limit_error(
additional_details=f"Hit Global Limit: Limit={global_max_parallel_requests}, current: {current_global_requests}"
)
# if below -> increment

View file

@ -326,6 +326,27 @@ class TestProxyHooksActuallyRaiseProxyRateLimitError:
# And it must still be catchable as HTTPException for FastAPI's
# default 429 dispatcher.
assert isinstance(e, HTTPException)
# Regression: the detail must include the supplied additional_details
# and must not stringify a None placeholder.
assert "key-over-rpm" in e.detail
assert "None" not in e.detail
def test_parallel_request_limiter_v1_helper_detail_omits_none(self):
"""Regression for the dead-variable / None-interpolation bug flagged
in code review: calling ``raise_rate_limit_error()`` without
``additional_details`` must NOT produce a detail string ending in
' None'."""
from unittest.mock import MagicMock
from litellm.proxy.hooks.parallel_request_limiter import (
_PROXY_MaxParallelRequestsHandler,
)
handler = _PROXY_MaxParallelRequestsHandler(internal_usage_cache=MagicMock())
with pytest.raises(ProxyRateLimitError) as exc_info:
handler.raise_rate_limit_error()
assert exc_info.value.detail == "Max parallel request limit reached"
assert "None" not in exc_info.value.detail
def test_parallel_request_limiter_v3_handle_rate_limit_error_raises(self):
"""v3 parallel_request_limiter's ``_handle_rate_limit_error`` must