mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-14 23:21:35 +00:00
fix(proxy): return 503 from CLI SSO endpoints when the configured Redis is unreachable
/sso/cli/start propagated a raw redis ConnectionError as a 500 because the per-IP rate-limit increment is the one cache path that re-raises. The Redis-authoritative flow write had the opposite problem: it silently swallowed the failure, stored the login session nowhere, and deferred the error to a confusing 400 at poll time. Both now fail closed with a deliberate 503 telling the operator the configured Redis cache is unreachable. RedisCache.set_cache gains an opt-in raise_on_error flag (default False, existing callers unchanged) so callers whose data lives only in Redis can hear about failed writes.
This commit is contained in:
parent
daf22ec871
commit
8fc7946a90
4 changed files with 108 additions and 11 deletions
|
|
@ -391,7 +391,7 @@ class RedisCache(BaseCache):
|
|||
# Fallback for unparseable versions (e.g., "v7.0.0", "latest")
|
||||
return DEFAULT_REDIS_MAJOR_VERSION
|
||||
|
||||
def set_cache(self, key, value, **kwargs):
|
||||
def set_cache(self, key, value, raise_on_error: bool = False, **kwargs):
|
||||
ttl = self.get_ttl(**kwargs)
|
||||
print_verbose(f"Set Redis Cache: key: {key}\nValue {value}\nttl={ttl}, redis_version={self.redis_version}")
|
||||
key = self.check_and_fix_namespace(key=key)
|
||||
|
|
@ -410,6 +410,8 @@ class RedisCache(BaseCache):
|
|||
except Exception as e:
|
||||
# NON blocking - notify users Redis is throwing an exception
|
||||
print_verbose(f"litellm.caching.caching: set() - Got exception from REDIS : {str(e)}")
|
||||
if raise_on_error:
|
||||
raise
|
||||
|
||||
def increment_cache(self, key, value: int, ttl: Optional[float] = None, **kwargs) -> int:
|
||||
_redis_client = self.redis_client
|
||||
|
|
|
|||
|
|
@ -148,6 +148,10 @@ _CLI_SSO_FLOW_CACHE_KEY_PREFIX = f"{CLI_SSO_SESSION_CACHE_KEY_PREFIX}:flow"
|
|||
_CLI_SSO_START_RATE_LIMIT_CACHE_KEY_PREFIX = f"{_CLI_SSO_FLOW_CACHE_KEY_PREFIX}:start_rate_limit"
|
||||
_CLI_SSO_START_RATE_LIMIT_WINDOW_SECONDS = 60
|
||||
_CLI_SSO_START_RATE_LIMIT_MAX_ATTEMPTS = 30
|
||||
_CLI_SSO_REDIS_UNAVAILABLE_DETAIL = (
|
||||
"CLI login requires the proxy's configured Redis cache, which is currently unreachable. "
|
||||
"Retry once Redis is healthy."
|
||||
)
|
||||
_CLI_SSO_USER_CODE_ALPHABET = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789"
|
||||
_CLI_SSO_LOGIN_ID_RE = re.compile(r"^cli-[A-Za-z0-9_-]{12,124}$")
|
||||
_CLI_SSO_USER_CODE_RE = re.compile(rf"^[{_CLI_SSO_USER_CODE_ALPHABET}]{{4}}-[{_CLI_SSO_USER_CODE_ALPHABET}]{{4}}$")
|
||||
|
|
@ -235,11 +239,18 @@ def _check_cli_sso_start_rate_limit(
|
|||
rate_limit_cache_key = _get_cli_sso_start_rate_limit_cache_key(
|
||||
request=request, use_x_forwarded_for=use_x_forwarded_for
|
||||
)
|
||||
current_attempts = cache.increment_cache(
|
||||
key=rate_limit_cache_key,
|
||||
value=1,
|
||||
ttl=_CLI_SSO_START_RATE_LIMIT_WINDOW_SECONDS,
|
||||
)
|
||||
try:
|
||||
current_attempts = cache.increment_cache(
|
||||
key=rate_limit_cache_key,
|
||||
value=1,
|
||||
ttl=_CLI_SSO_START_RATE_LIMIT_WINDOW_SECONDS,
|
||||
)
|
||||
except Exception as e:
|
||||
verbose_proxy_logger.error(
|
||||
"CLI SSO start rate limit check failed because the configured Redis cache is unreachable: %s",
|
||||
str(e),
|
||||
)
|
||||
raise HTTPException(status_code=503, detail=_CLI_SSO_REDIS_UNAVAILABLE_DETAIL) from e
|
||||
if current_attempts > _CLI_SSO_START_RATE_LIMIT_MAX_ATTEMPTS:
|
||||
raise HTTPException(
|
||||
status_code=429,
|
||||
|
|
@ -291,10 +302,23 @@ def _get_cli_sso_flow_or_raise(login_id: Optional[str], cache: DualCache) -> dic
|
|||
def _set_cli_sso_flow(login_id: str, cache: DualCache, flow: dict) -> None:
|
||||
cache_key = _get_cli_sso_flow_cache_key(login_id)
|
||||
redis_cache = cache.redis_cache
|
||||
if redis_cache is not None:
|
||||
redis_cache.set_cache(key=cache_key, value=json.dumps(flow), ttl=CLI_SSO_SESSION_TTL_SECONDS)
|
||||
else:
|
||||
if redis_cache is None:
|
||||
cache.set_cache(key=cache_key, value=flow, ttl=CLI_SSO_SESSION_TTL_SECONDS)
|
||||
return
|
||||
try:
|
||||
redis_cache.set_cache(
|
||||
key=cache_key,
|
||||
value=json.dumps(flow),
|
||||
ttl=CLI_SSO_SESSION_TTL_SECONDS,
|
||||
raise_on_error=True,
|
||||
)
|
||||
except Exception as e:
|
||||
verbose_proxy_logger.error(
|
||||
"CLI SSO login session for login_id=%s could not be written to the configured Redis cache: %s",
|
||||
login_id,
|
||||
str(e),
|
||||
)
|
||||
raise HTTPException(status_code=503, detail=_CLI_SSO_REDIS_UNAVAILABLE_DETAIL) from e
|
||||
|
||||
|
||||
def _verify_cli_sso_poll_secret(flow: dict, poll_secret: Optional[str]) -> bool:
|
||||
|
|
|
|||
|
|
@ -58,6 +58,22 @@ def test_delete_cache_applies_namespace(namespace, monkeypatch, redis_no_ping):
|
|||
mock_redis_client.delete.assert_called_once_with(expected_key)
|
||||
|
||||
|
||||
def test_set_cache_swallows_errors_by_default_and_raises_on_opt_in(monkeypatch, redis_no_ping):
|
||||
"""Sync set_cache is fire-and-forget for callers that tolerate a cold cache,
|
||||
but callers whose data lives only in Redis (e.g. CLI SSO login sessions)
|
||||
must be able to opt into hearing about a failed write."""
|
||||
monkeypatch.setenv("REDIS_HOST", "https://my-test-host")
|
||||
redis_cache = RedisCache()
|
||||
mock_redis_client = MagicMock()
|
||||
mock_redis_client.set.side_effect = ConnectionError("connection refused")
|
||||
redis_cache.redis_client = mock_redis_client
|
||||
|
||||
redis_cache.set_cache(key="some-key", value="some-value", ttl=60)
|
||||
|
||||
with pytest.raises(ConnectionError):
|
||||
redis_cache.set_cache(key="some-key", value="some-value", ttl=60, raise_on_error=True)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redis_client_init_with_socket_timeout(monkeypatch, redis_no_ping):
|
||||
monkeypatch.setenv("REDIS_HOST", "my-fake-host")
|
||||
|
|
|
|||
|
|
@ -2252,7 +2252,10 @@ class TestCLIKeyRegenerationFlow:
|
|||
_set_cli_sso_flow(login_id=login_id, cache=cache, flow=fresh_flow)
|
||||
|
||||
redis_cache.set_cache.assert_called_once_with(
|
||||
key=cache_key, value=json.dumps(fresh_flow), ttl=CLI_SSO_SESSION_TTL_SECONDS
|
||||
key=cache_key,
|
||||
value=json.dumps(fresh_flow),
|
||||
ttl=CLI_SSO_SESSION_TTL_SECONDS,
|
||||
raise_on_error=True,
|
||||
)
|
||||
cache.set_cache.assert_not_called()
|
||||
|
||||
|
|
@ -2288,7 +2291,7 @@ class TestCLIKeyRegenerationFlow:
|
|||
|
||||
redis_store: dict = {}
|
||||
redis_cache = MagicMock()
|
||||
redis_cache.set_cache.side_effect = lambda key, value, ttl: redis_store.__setitem__(
|
||||
redis_cache.set_cache.side_effect = lambda key, value, ttl, **kwargs: redis_store.__setitem__(
|
||||
key, str(value).encode("utf-8")
|
||||
)
|
||||
redis_cache.get_cache.side_effect = lambda key: RedisCache._get_cache_logic(
|
||||
|
|
@ -2363,6 +2366,58 @@ class TestCLIKeyRegenerationFlow:
|
|||
assert exc_info.value.status_code == 429
|
||||
mock_cache.set_cache.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cli_sso_start_returns_503_when_redis_unreachable(self):
|
||||
"""
|
||||
Regression: when a coordination Redis is configured but unreachable, the
|
||||
rate-limit increment used to propagate the raw ConnectionError as a 500.
|
||||
The endpoint must fail closed with a deliberate 503 instead.
|
||||
"""
|
||||
from litellm.proxy.management_endpoints.ui_sso import cli_sso_start
|
||||
|
||||
mock_request = MagicMock(spec=Request)
|
||||
mock_request.client = SimpleNamespace(host="127.0.0.1")
|
||||
mock_request.headers = {}
|
||||
mock_cache = MagicMock()
|
||||
mock_cache.increment_cache.side_effect = ConnectionError("Error 61 connecting to localhost:6379")
|
||||
|
||||
with (
|
||||
patch("litellm.proxy.proxy_server.user_api_key_cache", mock_cache),
|
||||
patch("litellm.proxy.proxy_server.cli_sso_session_cache", mock_cache),
|
||||
):
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await cli_sso_start(request=mock_request)
|
||||
|
||||
assert exc_info.value.status_code == 503
|
||||
assert "Redis" in exc_info.value.detail
|
||||
assert "unreachable" in exc_info.value.detail
|
||||
mock_cache.set_cache.assert_not_called()
|
||||
|
||||
def test_set_cli_sso_flow_raises_503_when_redis_write_fails(self):
|
||||
"""
|
||||
Regression: the Redis-authoritative flow write used to swallow connection
|
||||
errors, storing the login session nowhere and deferring the failure to a
|
||||
confusing 400 at poll time. A failed write must surface as a 503.
|
||||
"""
|
||||
from litellm.proxy.management_endpoints.ui_sso import _set_cli_sso_flow
|
||||
|
||||
redis_cache = MagicMock()
|
||||
redis_cache.set_cache.side_effect = ConnectionError("Error 61 connecting to localhost:6379")
|
||||
cache = MagicMock()
|
||||
cache.redis_cache = redis_cache
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
_set_cli_sso_flow(
|
||||
login_id="cli-redis_down_1234567890",
|
||||
cache=cache,
|
||||
flow={"poll_secret_hash": "hash"},
|
||||
)
|
||||
|
||||
assert exc_info.value.status_code == 503
|
||||
assert "Redis" in exc_info.value.detail
|
||||
assert redis_cache.set_cache.call_args.kwargs["raise_on_error"] is True
|
||||
cache.set_cache.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cli_sso_start_returns_verification_uri_complete_when_enabled(self):
|
||||
"""Test CLI SSO start returns a verification_uri_complete that round-trips the user_code only when the operator opts in"""
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue