mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-06 08:16:43 +00:00
fix(types): keep TagRateLimitEntry validation messages proxy-agnostic
litellm/types/router.py is imported by plain SDK users, not just the proxy; the previous ValueError messages for limit/key_ttl_seconds explained the proxy rate-limit hook's internal admission mechanics (atomic check-and-increment, read-only tokens/dollars check, cache TTL rollover), leaking implementation details across the SDK/proxy boundary. Move that mechanistic reasoning into code comments for future maintainers and keep the raised messages generic, per Greptile's finding on PR #38289. Adds regression tests asserting the three affected validators reject their invalid inputs without leaking proxy-internal enforcement jargon.
This commit is contained in:
parent
5df55369ff
commit
15ac18c742
2 changed files with 55 additions and 14 deletions
|
|
@ -237,18 +237,15 @@ class TagRateLimitEntry(BaseModel):
|
|||
# defeats the entry; reject it at config load time instead.
|
||||
if math.isnan(self.limit):
|
||||
raise ValueError("limit must not be NaN")
|
||||
# Positive infinity never rejects the checks that gate this limit; negative
|
||||
# infinity always does. Both silently defeat the entry.
|
||||
if math.isinf(self.limit):
|
||||
raise ValueError(
|
||||
"limit must be finite -- positive infinity makes admission never reject (current + increment "
|
||||
"> limit is always false), negative infinity makes it always reject every tagged request"
|
||||
)
|
||||
raise ValueError("limit must be finite")
|
||||
# Zero or negative makes every check that gates this limit either always
|
||||
# reject or never admit, silently blocking or admitting all matching traffic
|
||||
# instead of the likely intended config.
|
||||
if self.limit <= 0:
|
||||
raise ValueError(
|
||||
"limit must be a positive number -- zero or negative makes the atomic requests/concurrency "
|
||||
"check (current + increment > limit) reject every admission and the read-only tokens/dollars "
|
||||
"check (current < limit) never admit, silently blocking all matching traffic instead of the "
|
||||
"likely intended config"
|
||||
)
|
||||
raise ValueError("limit must be a positive number")
|
||||
return self
|
||||
|
||||
@model_validator(mode="after")
|
||||
|
|
@ -261,11 +258,10 @@ class TagRateLimitEntry(BaseModel):
|
|||
def _validate_key_ttl_seconds(self) -> "TagRateLimitEntry":
|
||||
if self.key_ttl_seconds is not None and self.key_ttl_seconds <= 0:
|
||||
raise ValueError("key_ttl_seconds must be a positive integer when set")
|
||||
# A shorter TTL than period_seconds expires the counter before its period
|
||||
# elapses, letting it reset early and exceed the limit.
|
||||
if self.key_ttl_seconds is not None and self.key_ttl_seconds < self.period_seconds:
|
||||
raise ValueError(
|
||||
"key_ttl_seconds must be at least period_seconds when set -- a shorter TTL expires the "
|
||||
"counter before its window rolls over, letting tagged traffic reset to zero and exceed the limit"
|
||||
)
|
||||
raise ValueError("key_ttl_seconds must be at least period_seconds when set")
|
||||
return self
|
||||
|
||||
@model_validator(mode="after")
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ from litellm.types.router import (
|
|||
Deployment,
|
||||
LiteLLM_Params,
|
||||
ModelInfo,
|
||||
TagRateLimitEntry,
|
||||
)
|
||||
from litellm.types.utils import CustomPricingLiteLLMParams, MirroredPricingParams
|
||||
|
||||
|
|
@ -89,3 +90,47 @@ def test_pricing_strings_are_coerced_to_float():
|
|||
def test_invalid_pricing_is_rejected():
|
||||
with pytest.raises(ValueError, match='validation error for ModelInfo'):
|
||||
ModelInfo(id="x", input_cost_per_token="free")
|
||||
|
||||
|
||||
# litellm/types/router.py is imported by plain SDK users, not just the proxy, so a
|
||||
# TagRateLimitEntry validation error should read as a generic config-validation
|
||||
# message and not describe the proxy rate-limit hook's internal admission mechanics.
|
||||
_INTERNAL_ENFORCEMENT_JARGON = (
|
||||
"admission",
|
||||
"tagged request",
|
||||
"tagged traffic",
|
||||
"check-and-increment",
|
||||
"read-only",
|
||||
"atomic",
|
||||
"window rolls over",
|
||||
)
|
||||
|
||||
|
||||
def _assert_message_has_no_internal_jargon(excinfo: pytest.ExceptionInfo) -> None:
|
||||
message = str(excinfo.value).lower()
|
||||
leaked = [term for term in _INTERNAL_ENFORCEMENT_JARGON if term in message]
|
||||
assert not leaked, f"validation message leaked internal enforcement jargon: {leaked}"
|
||||
|
||||
|
||||
def test_limit_infinite_rejected_without_internal_enforcement_jargon():
|
||||
with pytest.raises(ValueError, match="validation error for TagRateLimitEntry") as excinfo:
|
||||
TagRateLimitEntry(name="daily", tag_id="end_user_id", limit=float("inf"), period_seconds=60)
|
||||
_assert_message_has_no_internal_jargon(excinfo)
|
||||
|
||||
|
||||
def test_limit_non_positive_rejected_without_internal_enforcement_jargon():
|
||||
with pytest.raises(ValueError, match="validation error for TagRateLimitEntry") as excinfo:
|
||||
TagRateLimitEntry(name="daily", tag_id="end_user_id", limit=0, period_seconds=60)
|
||||
_assert_message_has_no_internal_jargon(excinfo)
|
||||
|
||||
|
||||
def test_key_ttl_seconds_shorter_than_period_rejected_without_internal_enforcement_jargon():
|
||||
with pytest.raises(ValueError, match="validation error for TagRateLimitEntry") as excinfo:
|
||||
TagRateLimitEntry(
|
||||
name="daily",
|
||||
tag_id="end_user_id",
|
||||
limit=500,
|
||||
period_seconds=86400,
|
||||
key_ttl_seconds=60,
|
||||
)
|
||||
_assert_message_has_no_internal_jargon(excinfo)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue