mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-12 23:01:41 +00:00
fix(proxy): return 5xx on DB infra errors during auth; reserve 401 for genuine auth failures (#29986)
(cherry picked from commit da9d64b4de)
This commit is contained in:
parent
0fdccebc3e
commit
7193461559
3 changed files with 291 additions and 0 deletions
|
|
@ -134,6 +134,16 @@ class UserAPIKeyAuthExceptionHandler:
|
|||
)
|
||||
elif isinstance(e, ProxyException):
|
||||
raise e
|
||||
if PrismaDBExceptionHandler.is_database_service_unavailable_error(e):
|
||||
raise ProxyException(
|
||||
message=(
|
||||
"Service Unavailable, the authentication database is "
|
||||
"temporarily unreachable. Please retry shortly."
|
||||
),
|
||||
type=ProxyErrorTypes.no_db_connection,
|
||||
param="None",
|
||||
code=status.HTTP_503_SERVICE_UNAVAILABLE,
|
||||
)
|
||||
raise ProxyException(
|
||||
message="Authentication Error, " + str(e),
|
||||
type=ProxyErrorTypes.auth_error,
|
||||
|
|
|
|||
|
|
@ -109,6 +109,92 @@ class PrismaDBExceptionHandler:
|
|||
return True
|
||||
return False
|
||||
|
||||
@staticmethod
|
||||
def is_prisma_engine_internal_error(e: Exception) -> bool:
|
||||
"""True iff ``e`` is a non-``PrismaError`` exception raised from inside
|
||||
prisma-client-py's query-engine layer.
|
||||
|
||||
During the instant a DB connection is torn down, the query engine can
|
||||
return a malformed error payload (``user_facing_error.meta`` is
|
||||
``null``). prisma-client-py's ``handle_response_errors`` then crashes
|
||||
with ``AttributeError: 'NoneType' object has no attribute 'get'``
|
||||
before it can raise the proper P1001 "can't reach database server"
|
||||
error. That AttributeError carries no connection keyword, so it can't
|
||||
be matched by message; identify it by its ``prisma.engine`` origin
|
||||
instead.
|
||||
|
||||
Recognized ``PrismaError`` subclasses are excluded: connectivity ones
|
||||
are already classified by type/keyword above, and data-layer ones
|
||||
(the DB IS reachable) must stay 401.
|
||||
"""
|
||||
import prisma
|
||||
|
||||
if isinstance(e, prisma.errors.PrismaError):
|
||||
return False
|
||||
tb = getattr(e, "__traceback__", None)
|
||||
while tb is not None:
|
||||
if tb.tb_frame.f_globals.get("__name__", "").startswith("prisma.engine"):
|
||||
return True
|
||||
tb = tb.tb_next
|
||||
return False
|
||||
|
||||
@staticmethod
|
||||
def is_database_service_unavailable_error(e: Exception) -> bool:
|
||||
"""True iff the exception means the database could not answer at the
|
||||
infrastructure level (connection refused, socket/interface failure,
|
||||
timeout) rather than a genuine auth failure (key not found) or a
|
||||
data-layer error (the DB IS reachable and rejected the data).
|
||||
|
||||
Auth must answer 401 only for a key the DB confirms is invalid. When
|
||||
the DB itself is unreachable, the request has to surface as 503 so
|
||||
callers retry instead of treating valid keys as invalid during an
|
||||
outage.
|
||||
|
||||
Note: prisma-client-py mislabels the P1001 "can't reach database
|
||||
server" connectivity failure as a ``DataError`` (a data-layer type),
|
||||
so a type-only check misses real outages. ``is_database_transport_error``
|
||||
keyword-matches the connection message and catches that masquerade,
|
||||
while genuine data errors (no connection keyword) correctly stay 401.
|
||||
|
||||
The Postgres "cached plan must not change result type" error is matched
|
||||
here, not in ``is_database_transport_error``: it is a transient stale-DB-
|
||||
state condition (not an invalid key), but the connection is healthy so it
|
||||
must not trigger a reconnect.
|
||||
|
||||
A non-``PrismaError`` raised from inside the prisma query engine (e.g.
|
||||
the ``AttributeError`` from ``handle_response_errors`` when the engine
|
||||
returns a malformed error payload mid-tear-down) is also treated as
|
||||
unavailable; see ``is_prisma_engine_internal_error``.
|
||||
"""
|
||||
import asyncio
|
||||
|
||||
if PrismaDBExceptionHandler.is_database_connection_error(e):
|
||||
return True
|
||||
if PrismaDBExceptionHandler.is_database_transport_error(e):
|
||||
return True
|
||||
if PrismaDBExceptionHandler.is_prisma_engine_internal_error(e):
|
||||
return True
|
||||
if "cached plan must not change result type" in str(e).lower():
|
||||
return True
|
||||
|
||||
# OSError already covers ConnectionError and (Py3.3+) TimeoutError.
|
||||
# asyncio.TimeoutError is a distinct class before Py3.11.
|
||||
if isinstance(e, (OSError, asyncio.TimeoutError)):
|
||||
return True
|
||||
|
||||
try:
|
||||
import asyncpg
|
||||
except ImportError:
|
||||
return False
|
||||
|
||||
return isinstance(
|
||||
e,
|
||||
(
|
||||
asyncpg.exceptions.PostgresConnectionError,
|
||||
asyncpg.exceptions.InterfaceError,
|
||||
),
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def handle_db_exception(e: Exception):
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -107,6 +107,201 @@ def test_is_database_connection_generic_errors():
|
|||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"error",
|
||||
[
|
||||
ConnectionError("connection refused"),
|
||||
TimeoutError("timed out"),
|
||||
OSError("network is unreachable"),
|
||||
asyncio.TimeoutError(),
|
||||
HTTPClientClosedError(),
|
||||
ClientNotConnectedError(),
|
||||
PrismaError("can't reach database server"),
|
||||
PrismaError(),
|
||||
],
|
||||
)
|
||||
def test_is_database_service_unavailable_error_infra_failures(error):
|
||||
"""Infrastructure-level failures (socket/connection/timeout, prisma
|
||||
transport, unknown PrismaError) mean the DB could not answer, so auth
|
||||
must surface 503 instead of treating a valid key as invalid."""
|
||||
assert PrismaDBExceptionHandler.is_database_service_unavailable_error(error) is True
|
||||
|
||||
|
||||
def test_is_database_service_unavailable_error_prisma_p1001_masquerades_as_dataerror():
|
||||
"""Real-world regression: prisma-client-py raises the P1001 "can't reach
|
||||
database server" connectivity failure as a DataError (a data-layer type).
|
||||
A type-only check would miss it and return 401 during a genuine outage;
|
||||
the message keyword must still classify it as service-unavailable -> 503."""
|
||||
p1001_as_dataerror = DataError(
|
||||
data={
|
||||
"user_facing_error": {
|
||||
"message": "Can't reach database server at `127.0.0.1`:`5499`",
|
||||
"meta": {"table": "t"},
|
||||
}
|
||||
}
|
||||
)
|
||||
assert (
|
||||
PrismaDBExceptionHandler.is_database_service_unavailable_error(
|
||||
p1001_as_dataerror
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
|
||||
def test_is_database_service_unavailable_error_cached_plan_escapes_as_503():
|
||||
"""Composes with the cached-plan retry: when that recovery fails and the
|
||||
Postgres "cached plan must not change result type" error escapes (raised by
|
||||
prisma as a data-layer RawQueryError), it is a transient stale-DB-state
|
||||
condition, not an invalid key, so it must classify as service-unavailable
|
||||
-> 503 rather than fall through to 401."""
|
||||
cached_plan_error = RawQueryError(
|
||||
data={
|
||||
"user_facing_error": {
|
||||
"message": "cached plan must not change result type",
|
||||
"meta": {"table": "t"},
|
||||
}
|
||||
}
|
||||
)
|
||||
assert (
|
||||
PrismaDBExceptionHandler.is_database_service_unavailable_error(
|
||||
cached_plan_error
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
|
||||
def test_is_database_service_unavailable_error_prisma_engine_malformed_payload():
|
||||
"""Real-world regression: at the instant the DB socket drops, the prisma
|
||||
query engine returns a malformed error payload (``user_facing_error.meta``
|
||||
is ``null``). prisma-client-py's ``handle_response_errors`` then crashes
|
||||
with ``AttributeError: 'NoneType' object has no attribute 'get'`` before it
|
||||
can raise the proper P1001 error. That bare AttributeError has no
|
||||
connection keyword, so without the prisma-engine-origin check it falls
|
||||
through to 401 on the first request of an outage. Reproduce the exact
|
||||
prisma crash and assert it classifies as service-unavailable -> 503."""
|
||||
from prisma.engine import utils as prisma_engine_utils
|
||||
|
||||
malformed_payload = [
|
||||
{
|
||||
"error": "Can't reach database server",
|
||||
"user_facing_error": {
|
||||
"error_code": "P1001",
|
||||
"message": "Can't reach database server at `localhost`:`5503`",
|
||||
"meta": None,
|
||||
},
|
||||
}
|
||||
]
|
||||
with pytest.raises(AttributeError) as exc_info:
|
||||
prisma_engine_utils.handle_response_errors(None, malformed_payload)
|
||||
|
||||
assert "no attribute 'get'" in str(exc_info.value)
|
||||
assert (
|
||||
PrismaDBExceptionHandler.is_database_service_unavailable_error(exc_info.value)
|
||||
is True
|
||||
)
|
||||
|
||||
|
||||
def test_is_prisma_engine_internal_error_excludes_application_attributeerror():
|
||||
"""The prisma-engine-origin check must stay narrow: a genuine AttributeError
|
||||
raised by application code (a real bug) must NOT be classified as
|
||||
service-unavailable, otherwise real bugs would silently become 503s."""
|
||||
|
||||
def application_bug():
|
||||
none_value = None
|
||||
return none_value.get("oops")
|
||||
|
||||
with pytest.raises(AttributeError) as exc_info:
|
||||
application_bug()
|
||||
|
||||
assert (
|
||||
PrismaDBExceptionHandler.is_prisma_engine_internal_error(exc_info.value)
|
||||
is False
|
||||
)
|
||||
assert (
|
||||
PrismaDBExceptionHandler.is_database_service_unavailable_error(exc_info.value)
|
||||
is False
|
||||
)
|
||||
|
||||
|
||||
def test_is_prisma_engine_internal_error_excludes_data_layer_prisma_error():
|
||||
"""A data-layer ``PrismaError`` (the DB IS reachable and rejected the data)
|
||||
must stay 401. These are always raised from prisma internals, so the check
|
||||
excludes any ``PrismaError`` by type before inspecting the traceback."""
|
||||
data_layer_error = UniqueViolationError(
|
||||
data={"user_facing_error": {"meta": {"table": "t"}}}
|
||||
)
|
||||
try:
|
||||
raise data_layer_error
|
||||
except UniqueViolationError as e:
|
||||
assert PrismaDBExceptionHandler.is_prisma_engine_internal_error(e) is False
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"error",
|
||||
[
|
||||
DataError(data={"user_facing_error": {"meta": {"table": "t"}}}),
|
||||
UniqueViolationError(data={"user_facing_error": {"meta": {"table": "t"}}}),
|
||||
RecordNotFoundError(data={"user_facing_error": {"meta": {"table": "t"}}}),
|
||||
Exception("some unrelated error"),
|
||||
ValueError("bad value"),
|
||||
],
|
||||
)
|
||||
def test_is_database_service_unavailable_error_excludes_non_infra(error):
|
||||
"""Data-layer errors (the DB IS reachable and answered) and generic
|
||||
non-DB errors must NOT be classified as service-unavailable, otherwise a
|
||||
genuine 401 would be masked as a transient 503."""
|
||||
assert (
|
||||
PrismaDBExceptionHandler.is_database_service_unavailable_error(error) is False
|
||||
)
|
||||
|
||||
|
||||
def test_is_database_service_unavailable_error_asyncpg(monkeypatch):
|
||||
"""asyncpg connection/interface errors map to service-unavailable. asyncpg
|
||||
is not a hard dependency, so inject a stand-in module to exercise the
|
||||
branch deterministically regardless of the install environment."""
|
||||
import sys
|
||||
import types
|
||||
|
||||
fake_asyncpg = types.ModuleType("asyncpg")
|
||||
fake_exceptions = types.ModuleType("asyncpg.exceptions")
|
||||
|
||||
class PostgresConnectionError(Exception):
|
||||
pass
|
||||
|
||||
class InterfaceError(Exception):
|
||||
pass
|
||||
|
||||
class UniqueViolationError(Exception): # data-layer, must stay False
|
||||
pass
|
||||
|
||||
fake_exceptions.PostgresConnectionError = PostgresConnectionError
|
||||
fake_exceptions.InterfaceError = InterfaceError
|
||||
fake_exceptions.UniqueViolationError = UniqueViolationError
|
||||
fake_asyncpg.exceptions = fake_exceptions
|
||||
|
||||
monkeypatch.setitem(sys.modules, "asyncpg", fake_asyncpg)
|
||||
monkeypatch.setitem(sys.modules, "asyncpg.exceptions", fake_exceptions)
|
||||
|
||||
assert (
|
||||
PrismaDBExceptionHandler.is_database_service_unavailable_error(
|
||||
PostgresConnectionError("connection reset")
|
||||
)
|
||||
is True
|
||||
)
|
||||
assert (
|
||||
PrismaDBExceptionHandler.is_database_service_unavailable_error(
|
||||
InterfaceError("connection was closed")
|
||||
)
|
||||
is True
|
||||
)
|
||||
assert (
|
||||
PrismaDBExceptionHandler.is_database_service_unavailable_error(
|
||||
UniqueViolationError("duplicate key")
|
||||
)
|
||||
is False
|
||||
)
|
||||
|
||||
|
||||
# Test should_allow_request_on_db_unavailable method
|
||||
@patch(
|
||||
"litellm.proxy.proxy_server.general_settings",
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue