From 748c2026d7b680e439f79308b192d13d8c823f0f Mon Sep 17 00:00:00 2001 From: "devin-ai-integration[bot]" <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 15:05:55 -0700 Subject: [PATCH] fix(proxy): word database 503s by whether the fault is transient (#39256) Permanent Prisma/query-engine faults keep the 503 status and no_db_connection type but stop claiming the database is temporarily unreachable. A permanent fault anywhere in the exception chain outranks the transport error that surfaced it. MCP bridge and DCR flows gain a faulted resolution state with matching wording. Resolves LIT-5208 Co-authored-by: yassin Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../mcp_server/auth/user_api_key_auth_mcp.py | 5 +- .../mcp_server/bridge_token_flow.py | 32 ++++++- .../mcp_server/gateway_dcr_flow.py | 27 ++++-- litellm/proxy/auth/auth_exception_handler.py | 4 +- litellm/proxy/db/exception_handler.py | 80 ++++++++++++++-- .../auth/test_user_api_key_auth_mcp.py | 38 ++++++++ .../mcp_server/test_discoverable_endpoints.py | 87 ++++++++++++++++++ .../mcp_server/test_gateway_dcr_flow.py | 29 ++++++ .../proxy/auth/test_auth_exception_handler.py | 91 +++++++++++++++++++ .../proxy/db/test_exception_handler.py | 83 +++++++++++++++++ 10 files changed, 449 insertions(+), 27 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py b/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py index 425f82794e6..66d4aedba06 100644 --- a/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py +++ b/litellm/proxy/_experimental/mcp_server/auth/user_api_key_auth_mcp.py @@ -1149,10 +1149,11 @@ class MCPRequestHandler: would miss a real outage wrapped inside it.""" from litellm.proxy.db.exception_handler import PrismaDBExceptionHandler - if PrismaDBExceptionHandler.is_database_service_unavailable_error_in_chain(e): + outage: Final = PrismaDBExceptionHandler.find_database_service_unavailable_error_in_chain(e) + if outage is not None: raise HTTPException( status_code=503, - detail="Service Unavailable, the authentication database is temporarily unreachable. Please retry shortly.", + detail=PrismaDBExceptionHandler.database_unavailable_message(outage), ) from None @staticmethod diff --git a/litellm/proxy/_experimental/mcp_server/bridge_token_flow.py b/litellm/proxy/_experimental/mcp_server/bridge_token_flow.py index 09a3703e904..35a30127e27 100644 --- a/litellm/proxy/_experimental/mcp_server/bridge_token_flow.py +++ b/litellm/proxy/_experimental/mcp_server/bridge_token_flow.py @@ -101,18 +101,29 @@ class _ResolvedKey: key: "UserAPIKeyAuth" -_KeyResolutionFailure = Literal["no_active_key", "unavailable", "unresolvable"] +_KeyResolutionFailure = Literal["no_active_key", "unavailable", "faulted", "unresolvable"] """Why a token request yielded no active litellm key, kept distinct so a caller statuses each truthfully instead of blaming the client for a gateway problem: - ``no_active_key``: none was presented, or the presented key is unknown / blocked / expired (the caller's request is at fault) - ``unavailable``: the auth database was transiently unreachable while resolving (retryable) +- ``faulted``: the auth database's query engine reported a fault that retrying will not clear (still a + 503, but the wording must not tell the operator to wait) - ``unresolvable``: the gateway cannot resolve identity right now (no DB connection, or an unexpected error) -- a gateway fault, not the caller's The classification mirrors admission's ``_reload_admitted_key`` so the mint (ingress) and admission (egress) never disagree on the status of the same outage.""" +def _database_failure(exc: Exception) -> Literal["unavailable", "faulted"]: + from litellm.proxy.db.exception_handler import ( # noqa: PLC0415 # inline import avoids a module-load circular import + PrismaDBExceptionHandler, + ) + + fault: Final = PrismaDBExceptionHandler.find_database_service_unavailable_error_in_chain(exc) or exc + return "faulted" if PrismaDBExceptionHandler.is_permanent_database_fault(fault) else "unavailable" + + async def _resolve_active_litellm_key(request: Request) -> "_ResolvedKey | _KeyResolutionFailure": """Resolve the presented litellm key to an active key record, or say precisely why not. @@ -170,7 +181,7 @@ async def _reload_active_key_by_hash(key_hash: str) -> "_ResolvedKey | _KeyResol return "no_active_key" except Exception as exc: # noqa: BLE001 # classify: a DB outage is retryable, anything else is an opaque gateway fault if PrismaDBExceptionHandler.is_database_service_unavailable_error(exc): - return "unavailable" + return _database_failure(exc) verbose_logger.debug( "_reload_active_key_by_hash: unexpected key-resolution error (%s)", type(exc).__name__, @@ -225,8 +236,9 @@ async def load_active_user_by_id(user_id: str) -> "LiteLLM_UserTable | _KeyResol except (ProxyException, HTTPException): return "no_active_key" except Exception as exc: # noqa: BLE001 # a DB outage is retryable; a missing user (get_user_object's wrapped ValueError) or any other resolution failure fails closed as no_active_key, never a 500 - if PrismaDBExceptionHandler.is_database_service_unavailable_error_in_chain(exc): - return "unavailable" + outage: Final = PrismaDBExceptionHandler.find_database_service_unavailable_error_in_chain(exc) + if outage is not None: + return _database_failure(outage) verbose_logger.debug("_reload_active_user_by_id: user-resolution error (%s)", type(exc).__name__) return "no_active_key" if user_object is None: @@ -383,6 +395,7 @@ _BridgeMintError = Literal[ "no_identity", "invalid_refresh", "identity_unavailable", + "identity_faulted", "identity_unresolvable", "not_configured", "no_upstream_token", @@ -433,6 +446,13 @@ def _bridge_mint_error_response(error: _BridgeMintError) -> JSONResponse: "temporarily_unavailable", "the authentication database is temporarily unreachable; retry shortly", ) + case "identity_faulted": + status, code, desc = ( + 503, + "temporarily_unavailable", + "the authentication database reported a fault that is not a transient outage; " + "retrying will not help until the gateway deployment is repaired", + ) case "identity_unresolvable": status, code, desc = ( 500, @@ -485,6 +505,8 @@ def _key_resolution_failure_to_mint_error(failure: _KeyResolutionFailure) -> _Br return "no_identity" case "unavailable": return "identity_unavailable" + case "faulted": + return "identity_faulted" case "unresolvable": return "identity_unresolvable" case _: @@ -569,6 +591,8 @@ def _refresh_key_failure_to_mint_error(failure: _KeyResolutionFailure) -> _Bridg return "invalid_refresh" case "unavailable": return "identity_unavailable" + case "faulted": + return "identity_faulted" case "unresolvable": return "identity_unresolvable" case _: diff --git a/litellm/proxy/_experimental/mcp_server/gateway_dcr_flow.py b/litellm/proxy/_experimental/mcp_server/gateway_dcr_flow.py index a43e762a456..c7b0045dde5 100644 --- a/litellm/proxy/_experimental/mcp_server/gateway_dcr_flow.py +++ b/litellm/proxy/_experimental/mcp_server/gateway_dcr_flow.py @@ -150,11 +150,18 @@ _CLIENT_RECORD_DEBUG_KEY: Final = "gateway_dcr_client" _CONNECT_FLOW_DEBUG_KEY: Final = "gateway_connect_flow" _AUTH_CODE_DEBUG_KEY: Final = "gateway_authorization_code" -ReloadUserFailure = Literal["unresolvable", "unavailable", "no_active_key"] +ReloadUserFailure = Literal["unresolvable", "unavailable", "faulted", "no_active_key"] ReloadUser = Callable[[str], Awaitable[ReloadUserFailure | None]] """Injected live-user revalidation (the token endpoint's mirror of admission): -``None`` means the user is active; ``unavailable`` is a retryable DB outage; anything -else fails the grant closed.""" +``None`` means the user is active; ``unavailable`` is a retryable DB outage; ``faulted`` is +a DB fault retrying will not clear (still 503, worded so nobody just waits); anything else +fails the grant closed.""" + +_DB_UNAVAILABLE_DESCRIPTION: Final = "the gateway database is unavailable; retry" +_DB_FAULTED_DESCRIPTION: Final = ( + "the gateway database reported a fault that is not a transient outage; " + "retrying will not help until the gateway deployment is repaired" +) PROXY_API_AUDIENCE: Final[SessionAudience] = "proxy_api" """The audience a native client (``lite login --pkce``, a Go CLI) asks for by sending the @@ -659,7 +666,9 @@ def _set_flow_cookie(response: Response, request: Request, handle: str, flow: _C def _consent_lookup_failure_response(failure: ReloadUserFailure) -> Response: match failure: case "unavailable": - return _oauth_error(503, "temporarily_unavailable", "the gateway database is unavailable; retry") + return _oauth_error(503, "temporarily_unavailable", _DB_UNAVAILABLE_DESCRIPTION) + case "faulted": + return _oauth_error(503, "temporarily_unavailable", _DB_FAULTED_DESCRIPTION) case "unresolvable": return _oauth_error(500, "server_error", "the gateway is not configured to resolve users") case "no_active_key": @@ -962,7 +971,9 @@ def _reload_failure_response(failure: ReloadUserFailure) -> Response: ``ReloadUserFailure`` member is a type error here rather than silently 400ing.""" match failure: case "unavailable": - return _oauth_error(503, "temporarily_unavailable", "the gateway database is unavailable; retry") + return _oauth_error(503, "temporarily_unavailable", _DB_UNAVAILABLE_DESCRIPTION) + case "faulted": + return _oauth_error(503, "temporarily_unavailable", _DB_FAULTED_DESCRIPTION) case "unresolvable": return _oauth_error(500, "server_error", "the gateway is not configured to resolve users") case "no_active_key": @@ -981,7 +992,7 @@ def _mint_failure_response(failure: ProxyCredentialMintFailure) -> Response: return _oauth_error( 400, "invalid_grant", "this user belongs to a team; sign in again and pick the team for this credential" ) - case "unavailable" | "unresolvable" | "no_active_key": + case "unavailable" | "faulted" | "unresolvable" | "no_active_key": return _reload_failure_response(failure) case _: assert_never(failure) @@ -1297,8 +1308,8 @@ async def introspect_gateway_token( if peeked == "claimed": return _inactive_introspection_response() failure: Final = await reload_user(opened.principal.user_id) - if failure == "unavailable": - return _oauth_error(503, "temporarily_unavailable", "the gateway database is unavailable; retry") + if failure == "unavailable" or failure == "faulted": + return _reload_failure_response(failure) if failure is not None: return _inactive_introspection_response() return _active_introspection_response(opened) diff --git a/litellm/proxy/auth/auth_exception_handler.py b/litellm/proxy/auth/auth_exception_handler.py index 64878a480a7..b36c8a038fc 100644 --- a/litellm/proxy/auth/auth_exception_handler.py +++ b/litellm/proxy/auth/auth_exception_handler.py @@ -61,9 +61,7 @@ def _as_proxy_exception(e: Exception) -> ProxyException: return e if PrismaDBExceptionHandler.is_database_service_unavailable_error(e): return ProxyException( - message=( - "Service Unavailable, the authentication database is temporarily unreachable. Please retry shortly." - ), + message=PrismaDBExceptionHandler.database_unavailable_message(e), type=ProxyErrorTypes.no_db_connection, param="None", code=status.HTTP_503_SERVICE_UNAVAILABLE, diff --git a/litellm/proxy/db/exception_handler.py b/litellm/proxy/db/exception_handler.py index 7c0aab948b6..ef1c4a66203 100644 --- a/litellm/proxy/db/exception_handler.py +++ b/litellm/proxy/db/exception_handler.py @@ -1,4 +1,4 @@ -from collections.abc import Awaitable, Callable +from collections.abc import Awaitable, Callable, Iterator from typing import Any, Final, TypeVar from litellm._logging import verbose_proxy_logger @@ -9,10 +9,32 @@ from litellm.proxy._types import ( ) from litellm.secret_managers.main import str_to_bool -# Bounds the __cause__/__context__ walk in is_database_service_unavailable_error_in_chain. +# Bounds the __cause__/__context__ walk in find_database_service_unavailable_error_in_chain. # Real exception chains are a few links deep; the cap also makes the walk cycle-safe. _MAX_EXCEPTION_CHAIN_DEPTH: Final = 20 +_TRANSIENT_DB_UNAVAILABLE_MESSAGE: Final = ( + "Service Unavailable, the authentication database is temporarily unreachable. Please retry shortly." +) + + +def _exception_chain(e: BaseException) -> Iterator[BaseException]: + current = e # rebind-ok: advances one link per iteration of the bounded walk + for _ in range(_MAX_EXCEPTION_CHAIN_DEPTH): + yield current + following = current.__cause__ or current.__context__ + if following is None: + return + current = following + + +def _database_service_unavailable_errors(e: BaseException) -> tuple[Exception, ...]: + return tuple( + link + for link in _exception_chain(e) + if isinstance(link, Exception) and PrismaDBExceptionHandler.is_database_service_unavailable_error(link) + ) + def _exception_types(*candidates: object) -> tuple[type[BaseException], ...]: """Keep only the real exception classes among ``candidates``. @@ -279,6 +301,51 @@ class PrismaDBExceptionHandler: ), ) + @staticmethod + def is_permanent_database_fault(e: Exception) -> bool: + """True for a service-unavailable failure that will not clear on its + own: an engine-layer ``PrismaError`` (missing or version-skewed engine + binary, engine error status, misused transaction) that is neither the + transient ``EngineConnectionError`` nor a reconnectable transport failure. + + Picks only the wording of a 503, never whether one is sent; + ``is_database_service_unavailable_error`` stays the status gate. + """ + if PrismaDBExceptionHandler.is_database_connection_error(e): + return False + if PrismaDBExceptionHandler.is_database_transport_error(e): + return False + return PrismaDBExceptionHandler.is_database_infrastructure_error(e) + + @staticmethod + def database_unavailable_message(e: Exception) -> str: + """The 503 detail for a service-unavailable database failure: retry + guidance for a transient outage, a pointer at the deployment for a + fault that retrying cannot fix. A permanent fault anywhere in the + exception chain wins, since the transport error that surfaced it is + not what blocks recovery.""" + fault: Final = PrismaDBExceptionHandler.find_database_service_unavailable_error_in_chain(e) or e + if not PrismaDBExceptionHandler.is_permanent_database_fault(fault): + return _TRANSIENT_DB_UNAVAILABLE_MESSAGE + return ( + "Service Unavailable, the authentication database query engine reported " + f"{type(fault).__name__}, which is not a transient outage and will not clear by retrying. " + "The proxy deployment needs attention." + ) + + @staticmethod + def find_database_service_unavailable_error_in_chain(e: BaseException) -> Exception | None: + """The exception in the ``__cause__`` / ``__context__`` chain that + ``is_database_service_unavailable_error`` accepts, or ``None``. Callers + that word a response by the kind of outage need the wrapped database + error itself, not just the fact that one is present. A permanent fault + outranks a transient one wherever it sits in the chain: a reconnect that + dies on a missing engine binary raises the transport error last, but the + binary is what keeps the database down.""" + outages: Final = _database_service_unavailable_errors(e) + permanent: Final = next(filter(PrismaDBExceptionHandler.is_permanent_database_fault, outages), None) + return permanent if permanent is not None else next(iter(outages), None) + @staticmethod def is_database_service_unavailable_error_in_chain(e: BaseException) -> bool: """Like ``is_database_service_unavailable_error`` but also walks the @@ -296,14 +363,7 @@ class PrismaDBExceptionHandler: The walk is depth-bounded, which also makes it cycle-safe. """ - current: BaseException | None = e - for _ in range(_MAX_EXCEPTION_CHAIN_DEPTH): - if not isinstance(current, Exception): - return False - if PrismaDBExceptionHandler.is_database_service_unavailable_error(current): - return True - current = current.__cause__ or current.__context__ - return False + return PrismaDBExceptionHandler.find_database_service_unavailable_error_in_chain(e) is not None @staticmethod def handle_db_exception(e: Exception): diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py b/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py index 0144fbb17dd..c8ea4867f2c 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/auth/test_user_api_key_auth_mcp.py @@ -6005,6 +6005,44 @@ class TestMCPDcrBridgeDelegateAdmission: await MCPRequestHandler.process_mcp_request(scope) assert exc_info.value.status_code == 503 + assert exc_info.value.detail == ( + "Service Unavailable, the authentication database is temporarily unreachable. Please retry shortly." + ) + + async def test_user_subject_envelope_permanent_db_fault_is_503_not_worded_as_transient(self): + """A query engine fault that never heals (a missing engine binary) still fails admission with 503, + but the detail must not call the database "temporarily unreachable" or ask the client to retry: the + DCR client would loop on a retry that can never succeed. The fault reaches the handler wrapped in + get_user_object's bare ValueError, so the wording has to be picked off the wrapped cause.""" + from prisma.engine.errors import BinaryNotFoundError + + envelope = self._mint_bridge_envelope(user_id="sso-user-7") + scope = { + "type": "http", + "method": "POST", + "path": "/mcp/bridge_delegate_server", + "headers": [(b"authorization", f"Bearer {envelope}".encode("latin-1"))], + } + with ( + patch( # test-quality-ok: isolate the MCP registry, same seam as the sibling admission tests + "litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager" + ) as mock_mgr, + patch( # test-quality-ok: the envelope opener reads master_key off the proxy module, no injection seam + "litellm.proxy.proxy_server.master_key", self._MASTER_KEY + ), + self._patch_user_reload( + side_effect=self._wrapped_user_lookup_error(BinaryNotFoundError("query engine binary not found")) + ), + ): + mock_mgr.get_mcp_server_by_name.return_value = self._bridge_delegate_server() + with pytest.raises(HTTPException) as exc_info: + await MCPRequestHandler.process_mcp_request(scope) + + assert exc_info.value.status_code == 503 + assert "temporarily unreachable" not in exc_info.value.detail + assert "retry shortly" not in exc_info.value.detail.lower() + assert "BinaryNotFoundError" in exc_info.value.detail + assert "will not clear by retrying" in exc_info.value.detail async def test_user_subject_envelope_scim_deactivated_user_fails_closed_401(self): """SCIM-deactivating the envelope's user revokes it immediately: the reloaded user carries diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py index 598e9276423..588eba4adb7 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_discoverable_endpoints.py @@ -6432,6 +6432,23 @@ async def test_bridge_mint_db_outage_is_503_before_upstream(): response, post = await _prepare_only_bridge_exchange("unavailable") assert response.status_code == 503 assert json.loads(response.body)["error"] == "temporarily_unavailable" + assert "retry shortly" in json.loads(response.body)["error_description"] + post.assert_not_called() + + +@pytest.mark.asyncio +async def test_bridge_mint_permanent_db_fault_is_503_without_retry_advice(): + """A query engine fault that never heals is still a 503 (the gateway is at fault, not the client), but + the description must not tell the client the database is temporarily unreachable and to retry: that + sends an operator to wait out an outage that is not one. The code stays temporarily_unavailable, the + only RFC 6749 error a client treats as a server-side 503.""" + response, post = await _prepare_only_bridge_exchange("faulted") + assert response.status_code == 503 + body = json.loads(response.body) + assert body["error"] == "temporarily_unavailable" + assert "temporarily unreachable" not in body["error_description"] + assert "retry shortly" not in body["error_description"] + assert "not a transient outage" in body["error_description"] post.assert_not_called() @@ -7143,6 +7160,56 @@ async def test_resolve_active_litellm_key_db_outage_is_unavailable(proxy_globals assert await _resolve_active_litellm_key(request) == "unavailable" +@pytest.mark.asyncio +async def test_resolve_active_litellm_key_permanent_engine_fault_is_faulted(proxy_globals): + """A query engine that is missing or version-skewed cannot resolve any key until the deployment is + repaired, so the resolver reports "faulted" (still statused 503 by the mint) rather than "unavailable", + whose wording promises the outage is transient and asks the client to retry.""" + from prisma.engine.errors import BinaryNotFoundError + + from litellm.proxy._experimental.mcp_server.bridge_token_flow import ( + _resolve_active_litellm_key, + ) + from litellm.proxy.common_utils.user_api_key_cache import UserApiKeyCache + + class _FaultedPrisma: + async def get_data(self, token, table_name, parent_otel_span=None, proxy_logging_obj=None): + raise BinaryNotFoundError("query engine binary not found") + + proxy_globals.user_api_key_cache = UserApiKeyCache() + proxy_globals.prisma_client = _FaultedPrisma() + + request = _token_request({"x-litellm-api-key": "sk-during-engine-fault"}) + assert await _resolve_active_litellm_key(request) == "faulted" + + +@pytest.mark.asyncio +async def test_resolve_active_litellm_key_transport_error_over_permanent_fault_is_faulted(proxy_globals): + """A reconnect that dies on a missing engine binary raises the transport error last, with the + BinaryNotFoundError as __context__. The binary is what blocks recovery, so the key read is "faulted", + not the "unavailable" that the outer ConnectError alone would suggest.""" + import httpx + from prisma.engine.errors import BinaryNotFoundError + + from litellm.proxy._experimental.mcp_server.bridge_token_flow import ( + _resolve_active_litellm_key, + ) + from litellm.proxy.common_utils.user_api_key_cache import UserApiKeyCache + + class _ReconnectFailedPrisma: + async def get_data(self, token, table_name, parent_otel_span=None, proxy_logging_obj=None): + try: + raise BinaryNotFoundError("query engine binary not found") + except BinaryNotFoundError: + raise httpx.ConnectError("All connection attempts failed") + + proxy_globals.user_api_key_cache = UserApiKeyCache() + proxy_globals.prisma_client = _ReconnectFailedPrisma() + + request = _token_request({"x-litellm-api-key": "sk-during-failed-reconnect"}) + assert await _resolve_active_litellm_key(request) == "faulted" + + @pytest.mark.asyncio async def test_resolve_active_litellm_key_no_database_is_unresolvable(proxy_globals): """With no database connection configured the gateway cannot verify the presented key at all, so @@ -7214,6 +7281,26 @@ async def test_reload_active_user_by_id_db_outage_is_unavailable(proxy_globals): assert await _reload_active_user_by_id("sso-user-7") == "unavailable" +@pytest.mark.asyncio +async def test_reload_active_user_by_id_permanent_engine_fault_is_faulted(proxy_globals): + """A permanent query engine fault while re-validating the user on refresh is "faulted", not + "unavailable": both are 503s, but only the transient one may tell the client to retry. get_user_object + wraps the fault in a bare ValueError, so the classification has to read the wrapped cause.""" + from prisma.engine.errors import MismatchedVersionsError + + from litellm.proxy._experimental.mcp_server.bridge_token_flow import _reload_active_user_by_id + from litellm.proxy.common_utils.user_api_key_cache import UserApiKeyCache + + proxy_globals.user_api_key_cache = UserApiKeyCache() + proxy_globals.prisma_client = object() + + with patch( # test-quality-ok: get_user_object is the DB seam that wraps the fault; same patch as the outage sibling + "litellm.proxy.auth.auth_checks.get_user_object", + new=AsyncMock(side_effect=_wrapped_user_lookup_error(MismatchedVersionsError(expected="1", got="2"))), + ): + assert await _reload_active_user_by_id("sso-user-7") == "faulted" + + @pytest.mark.asyncio async def test_token_endpoint_uses_client_secret_basic_when_configured(): """LIT-4091: a server with token_endpoint_auth_method=client_secret_basic must send the diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_gateway_dcr_flow.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_gateway_dcr_flow.py index 32a3f70c357..1670370f082 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_gateway_dcr_flow.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_gateway_dcr_flow.py @@ -426,6 +426,7 @@ async def test_token_rejects_expired_code_and_missing_configuration(): [ ("no_active_key", 400, "invalid_grant"), ("unavailable", 503, "temporarily_unavailable"), + ("faulted", 503, "temporarily_unavailable"), ("unresolvable", 500, "server_error"), ], ) @@ -460,6 +461,25 @@ async def test_token_gates_on_live_user_revalidation(failure, expected_status, e assert json.loads(response.body)["error"] == expected_error +def test_permanent_db_fault_503_does_not_promise_a_retry_will_help(): + """Both DB failures are 503 temporarily_unavailable (the only OAuth error a client reads as a + server-side outage), so the description is the one place the two are told apart: a transient outage + says retry, a fault that never heals must say retrying will not help and point at the deployment.""" + from litellm.proxy._experimental.mcp_server.gateway_dcr_flow import ( + _consent_lookup_failure_response, + _mint_failure_response, + _reload_failure_response, + ) + + for render in (_reload_failure_response, _consent_lookup_failure_response, _mint_failure_response): + transient = json.loads(render("unavailable").body)["error_description"] + faulted = json.loads(render("faulted").body)["error_description"] + assert transient == "the gateway database is unavailable; retry" + assert "retry" not in faulted.replace("retrying will not help", "") + assert "not a transient outage" in faulted + assert "retrying will not help" in faulted + + @pytest.mark.asyncio async def test_flow_is_single_use_shared_cache_rejects_second_complete(): """A double-submit of the finish step mints only ONE code: the second complete over the @@ -1250,6 +1270,7 @@ async def test_native_authorize_refuses_a_hosted_redirect_for_the_proxy_api(): "failure, status, error", [ ("unavailable", 503, "temporarily_unavailable"), + ("faulted", 503, "temporarily_unavailable"), ("unresolvable", 500, "server_error"), ("no_active_key", 403, "access_denied"), ], @@ -1424,6 +1445,7 @@ async def test_native_code_without_a_minter_is_refused_server_side(): ("team_required", 400, "invalid_grant"), ("no_active_key", 400, "invalid_grant"), ("unavailable", 503, "temporarily_unavailable"), + ("faulted", 503, "temporarily_unavailable"), ("unresolvable", 500, "server_error"), ], ) @@ -1805,5 +1827,12 @@ async def test_introspect_fails_closed_on_dead_user_and_503s_on_outage(): status, body = await _introspect(minted.token.get_secret_value(), reload_user=_reload_user_outage) assert (status, body["error"]) == (503, "temporarily_unavailable") + async def _reload_user_faulted(user_id: str): + return "faulted" + + status, body = await _introspect(minted.token.get_secret_value(), reload_user=_reload_user_faulted) + assert (status, body["error"]) == (503, "temporarily_unavailable") + assert "not a transient outage" in body["error_description"] + status, body = await _introspect(minted.token.get_secret_value(), master_key=None) assert (status, body["error"]) == (500, "server_error") diff --git a/tests/test_litellm/proxy/auth/test_auth_exception_handler.py b/tests/test_litellm/proxy/auth/test_auth_exception_handler.py index 90be51cfa5b..21e0b83791f 100644 --- a/tests/test_litellm/proxy/auth/test_auth_exception_handler.py +++ b/tests/test_litellm/proxy/auth/test_auth_exception_handler.py @@ -9,6 +9,7 @@ from prisma import errors as prisma_errors from prisma.engine.errors import ( BinaryNotFoundError, EngineConnectionError, + EngineRequestError, MismatchedVersionsError, ) from prisma.errors import ( @@ -32,6 +33,12 @@ from litellm.proxy._types import ProxyErrorTypes, ProxyException, UserAPIKeyAuth from litellm.proxy.auth.auth_exception_handler import UserAPIKeyAuthExceptionHandler +class _EngineHttp500: + """The response half of an EngineRequestError: the query engine answered a request with HTTP 500.""" + + status = 500 + + @pytest.mark.asyncio @pytest.mark.parametrize( "db_error", @@ -113,6 +120,90 @@ async def test_handle_authentication_error_permanent_fault_gets_no_fallback_iden assert exc_info.value.code == str(status.HTTP_503_SERVICE_UNAVAILABLE) +@pytest.mark.asyncio +@pytest.mark.parametrize( + "prisma_error", + [ + pytest.param(BinaryNotFoundError("query engine binary not found"), id="BinaryNotFoundError"), + pytest.param(MismatchedVersionsError(expected="1", got="2"), id="MismatchedVersionsError"), + pytest.param(EngineRequestError(_EngineHttp500(), "query engine crashed"), id="EngineRequestError"), + pytest.param(PrismaError(), id="bare_PrismaError"), + ], +) +async def test_handle_authentication_error_permanent_fault_503_is_not_worded_as_transient(prisma_error): + """The 503 for a fault that never heals must not say the database is + "temporarily unreachable" and ask the caller to retry. The status is right + (the service is at fault) but that wording sends the operator to wait out an + outage that is not one, so the message has to say retrying will not help and + name the engine fault.""" + handler = UserAPIKeyAuthExceptionHandler() + + with patch( # test-quality-ok: the handler reads general_settings off the proxy module, no injection seam + "litellm.proxy.proxy_server.general_settings", {"allow_requests_on_db_unavailable": False} + ): + with pytest.raises(ProxyException) as exc_info: + await handler._handle_authentication_error(prisma_error, MagicMock(), {}, "/test", None, "test-key") + + assert exc_info.value.code == str(status.HTTP_503_SERVICE_UNAVAILABLE) + assert exc_info.value.type == ProxyErrorTypes.no_db_connection + assert "temporarily unreachable" not in exc_info.value.message + assert "retry shortly" not in exc_info.value.message.lower() + assert "will not clear by retrying" in exc_info.value.message + assert type(prisma_error).__name__ in exc_info.value.message + + +@pytest.mark.asyncio +async def test_handle_authentication_error_transport_error_raised_over_a_permanent_fault_names_the_fault(): + """A reconnect attempt that fails because the engine binary is missing surfaces as a transport + error with the BinaryNotFoundError as __context__. The response must describe the binary, which is + what keeps the database down, rather than promise the connection will come back.""" + try: + raise BinaryNotFoundError("query engine binary not found") + except BinaryNotFoundError: + try: + raise httpx.ConnectError("All connection attempts failed") + except httpx.ConnectError as surfaced: + transport_over_fault = surfaced + handler = UserAPIKeyAuthExceptionHandler() + + with patch( # test-quality-ok: the handler reads general_settings off the proxy module, no injection seam + "litellm.proxy.proxy_server.general_settings", {"allow_requests_on_db_unavailable": False} + ): + with pytest.raises(ProxyException) as exc_info: + await handler._handle_authentication_error(transport_over_fault, MagicMock(), {}, "/test", None, "k") + + assert exc_info.value.code == str(status.HTTP_503_SERVICE_UNAVAILABLE) + assert "temporarily unreachable" not in exc_info.value.message + assert "BinaryNotFoundError" in exc_info.value.message + assert "will not clear by retrying" in exc_info.value.message + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "db_error", + [ + pytest.param(httpx.ConnectError("All connection attempts failed"), id="ConnectError"), + pytest.param(EngineConnectionError(), id="EngineConnectionError"), + pytest.param(PrismaError("can't reach database server"), id="P1001_text"), + ], +) +async def test_handle_authentication_error_transient_outage_503_keeps_retry_wording(db_error): + """A genuine outage is expected to come back, so its 503 keeps telling the + caller the database is temporarily unreachable and to retry.""" + handler = UserAPIKeyAuthExceptionHandler() + + with patch( # test-quality-ok: the handler reads general_settings off the proxy module, no injection seam + "litellm.proxy.proxy_server.general_settings", {"allow_requests_on_db_unavailable": False} + ): + with pytest.raises(ProxyException) as exc_info: + await handler._handle_authentication_error(db_error, MagicMock(), {}, "/test", None, "test-key") + + assert exc_info.value.code == str(status.HTTP_503_SERVICE_UNAVAILABLE) + assert exc_info.value.message == ( + "Service Unavailable, the authentication database is temporarily unreachable. Please retry shortly." + ) + + @pytest.mark.asyncio @pytest.mark.parametrize( "prisma_error", diff --git a/tests/test_litellm/proxy/db/test_exception_handler.py b/tests/test_litellm/proxy/db/test_exception_handler.py index 43e241c50a6..c685d778c0e 100644 --- a/tests/test_litellm/proxy/db/test_exception_handler.py +++ b/tests/test_litellm/proxy/db/test_exception_handler.py @@ -8,6 +8,7 @@ import httpx import pytest from fastapi import HTTPException, Request from prisma import errors as prisma_errors +from prisma.engine.errors import BinaryNotFoundError, EngineConnectionError from prisma.errors import ( ClientNotConnectedError, DataError, @@ -318,6 +319,43 @@ def test_is_database_service_unavailable_error_in_chain_sees_through_wrapping(): assert PrismaDBExceptionHandler.is_database_service_unavailable_error_in_chain(ValueError("nope")) is False +def test_find_database_service_unavailable_error_in_chain_returns_the_wrapped_outage_itself(): + """Wording a 503 by the kind of outage needs the wrapped database error, not the ValueError + get_user_object wrapped it in, so the finder must hand back the inner exception.""" + outage = _wrapped_like_get_user_object(ConnectionError("can't reach database server")) + found = PrismaDBExceptionHandler.find_database_service_unavailable_error_in_chain(outage) + assert isinstance(found, ConnectionError) + assert found is outage.__context__ + missing_user = _wrapped_like_get_user_object(Exception()) + assert PrismaDBExceptionHandler.find_database_service_unavailable_error_in_chain(missing_user) is None + + +def _raised_while_handling(inner, outer): + try: + raise inner + except BaseException: + try: + raise outer + except BaseException as surfaced: + return surfaced + + +def test_permanent_fault_outranks_the_transient_error_that_surfaced_it(): + """A reconnect that dies on a missing engine binary raises the transport error last, with the + BinaryNotFoundError left as __context__. The binary is what keeps the database down, so both the + finder and the 503 wording must pick it over the outer transient error, whichever way they nest.""" + permanent = BinaryNotFoundError("query engine binary not found") + transient_over_permanent = _raised_while_handling(permanent, httpx.ConnectError("connection refused")) + permanent_over_transient = _raised_while_handling(httpx.ConnectError("connection refused"), permanent) + + for chain in (transient_over_permanent, permanent_over_transient): + assert PrismaDBExceptionHandler.find_database_service_unavailable_error_in_chain(chain) is permanent + message = PrismaDBExceptionHandler.database_unavailable_message(chain) + assert "BinaryNotFoundError" in message + assert "will not clear by retrying" in message + assert "temporarily unreachable" not in message + + def test_is_database_service_unavailable_error_in_chain_terminates_on_a_cause_cycle(): """The walk must terminate on a pathological __cause__ cycle rather than hang. Neither link is an outage, so the bounded walk returns False instead of looping forever.""" @@ -509,6 +547,51 @@ def test_permanent_prisma_faults_are_still_reported_as_service_problems(prisma_e assert PrismaDBExceptionHandler.is_database_service_unavailable_error(prisma_error) is True +RECONNECTABLE_CLIENT_STATE_FAULTS = (prisma_errors.ClientNotConnectedError, prisma_errors.HTTPClientClosedError) + + +@pytest.mark.parametrize("prisma_error", PERMANENT_PRISMA_FAULTS) +def test_permanent_prisma_faults_are_worded_as_not_retryable(prisma_error): + """A 503 for a fault that never heals must not tell the operator to wait. + + The status stays 503 (the service is at fault), but the message has to say + the outage is not transient and name the engine fault, or an operator + watching a version-skewed engine keeps retrying a request that can never + succeed. The two client-state faults a reconnect can repair keep the retry + wording.""" + reconnectable = isinstance(prisma_error, RECONNECTABLE_CLIENT_STATE_FAULTS) + message = PrismaDBExceptionHandler.database_unavailable_message(prisma_error) + + assert PrismaDBExceptionHandler.is_permanent_database_fault(prisma_error) is (not reconnectable) + assert message.startswith("Service Unavailable") + assert ("temporarily unreachable" in message) is reconnectable + assert ("Please retry shortly" in message) is reconnectable + assert ("will not clear by retrying" in message) is (not reconnectable) + assert (type(prisma_error).__name__ in message) is (not reconnectable) + + +@pytest.mark.parametrize( + "transient_error", + [ + pytest.param(httpx.ConnectError("All connection attempts failed"), id="ConnectError"), + pytest.param(ConnectionError("connection refused"), id="ConnectionError"), + pytest.param(EngineConnectionError(), id="EngineConnectionError"), + pytest.param(prisma_errors.PrismaError("can't reach database server"), id="P1001_text"), + pytest.param( + ProxyException(message="no db", type=ProxyErrorTypes.no_db_connection, param=None, code=503), + id="ProxyException", + ), + ], +) +def test_transient_outages_keep_the_retry_wording(transient_error): + """A genuine outage is expected to come back, so the retry guidance is the + right message and must not be replaced by the permanent-fault text.""" + assert PrismaDBExceptionHandler.is_permanent_database_fault(transient_error) is False + assert PrismaDBExceptionHandler.database_unavailable_message(transient_error) == ( + "Service Unavailable, the authentication database is temporarily unreachable. Please retry shortly." + ) + + @pytest.mark.parametrize( "transient_error", [