mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-06 02:48:13 +00:00
fix(proxy): preserve original transport error if reconnect itself raises
Greptile review on #26756 (P2): if `attempt_db_reconnect` itself raises (e.g. lock cancellation, timer error, unexpected internal failure), the original `httpx.ReadError` / transport error was lost — `failure_handler` and `db_exceptions` alerts then logged the reconnect exception instead of the actual DB transport problem, masking the root cause. Wrap the reconnect call in a try/except. On reconnect failure, re-raise the *original* `first_exc` and chain the reconnect error as `__cause__` so it remains visible for debuggability without becoming the primary exception observers see. Adds `test_call_with_db_reconnect_retry_preserves_original_error_when_reconnect_raises` asserting (a) the propagated exception is the original transport error and (b) the reconnect exception is attached as `__cause__`.
This commit is contained in:
parent
1c9c219a74
commit
aa2ef41200
2 changed files with 50 additions and 5 deletions
|
|
@ -232,11 +232,26 @@ async def call_with_db_reconnect_retry(
|
|||
first_exc,
|
||||
)
|
||||
|
||||
did_reconnect = await prisma_client.attempt_db_reconnect(
|
||||
reason=reason,
|
||||
timeout_seconds=resolved_timeout,
|
||||
lock_timeout_seconds=resolved_lock_timeout,
|
||||
)
|
||||
# Preserve the original transport error in telemetry. If
|
||||
# `attempt_db_reconnect` itself raises (e.g. lock cancellation, timer
|
||||
# error, unexpected internal failure), surfacing that exception
|
||||
# instead of `first_exc` would mask the actual DB transport problem
|
||||
# in `failure_handler` / `db_exceptions` alerts. Chain the reconnect
|
||||
# error as the cause for debuggability without losing the original.
|
||||
try:
|
||||
did_reconnect = await prisma_client.attempt_db_reconnect(
|
||||
reason=reason,
|
||||
timeout_seconds=resolved_timeout,
|
||||
lock_timeout_seconds=resolved_lock_timeout,
|
||||
)
|
||||
except Exception as reconnect_exc:
|
||||
verbose_proxy_logger.warning(
|
||||
"DB reconnect attempt raised; preserving original transport error. "
|
||||
"reason=%s reconnect_error=%s",
|
||||
reason,
|
||||
reconnect_exc,
|
||||
)
|
||||
raise first_exc from reconnect_exc
|
||||
if not did_reconnect:
|
||||
raise
|
||||
|
||||
|
|
|
|||
|
|
@ -223,3 +223,33 @@ async def test_call_with_db_reconnect_retry_uses_auth_defaults_when_unset():
|
|||
call_kwargs = client.attempt_db_reconnect.await_args.kwargs
|
||||
assert call_kwargs["timeout_seconds"] == 3.0
|
||||
assert call_kwargs["lock_timeout_seconds"] == 0.5
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_call_with_db_reconnect_retry_preserves_original_error_when_reconnect_raises():
|
||||
"""If `attempt_db_reconnect` itself raises (lock cancellation, timer
|
||||
error, unexpected internal failure), the helper must surface the
|
||||
*original* transport error to telemetry — not the reconnect exception.
|
||||
Otherwise `failure_handler` / `db_exceptions` alerts log the wrong
|
||||
error string and the actual DB transport problem becomes invisible.
|
||||
|
||||
The reconnect error is chained as the `__cause__` for debuggability."""
|
||||
client = MagicMock()
|
||||
reconnect_exc = RuntimeError("simulated reconnect lock cancellation")
|
||||
client.attempt_db_reconnect = AsyncMock(side_effect=reconnect_exc)
|
||||
client._db_auth_reconnect_timeout_seconds = 2.0
|
||||
client._db_auth_reconnect_lock_timeout_seconds = 0.1
|
||||
|
||||
original_exc = httpx.ReadError("transport blip")
|
||||
|
||||
async def _factory():
|
||||
raise original_exc
|
||||
|
||||
with pytest.raises(httpx.ReadError) as exc_info:
|
||||
await call_with_db_reconnect_retry(
|
||||
client, _factory, reason="reconnect_itself_raises"
|
||||
)
|
||||
|
||||
assert exc_info.value is original_exc
|
||||
assert exc_info.value.__cause__ is reconnect_exc
|
||||
client.attempt_db_reconnect.assert_awaited_once()
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue