mirror of
https://github.com/tiennm99/litellm.git
synced 2026-07-17 12:17:35 +00:00
fix(proxy): suppress deferred success log when post-call guardrail blocks
When a non-streaming request had any post-call guardrail registered, the proxy deferred the async-success logging closure until after post_call_success_hook ran. The finally block fired that closure even when the hook raised — the propagating HTTPException then routed through post_call_failure_hook → _handle_logging_proxy_only_error, which writes its own failure spend log via async_failure_handler. The result was two spend log rows per blocked request: one Success exposing the blocked LLM response and one Failure. Reproduces with both pre and post bedrock guardrails configured for a team when the post-call OUTPUT scan blocks the response. Gate the deferred closure on _exception_raised so the failure path remains the single source of truth for blocked requests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
7e57b15de2
commit
f6e4e106df
@@ -1242,19 +1242,24 @@ class ProxyBaseLLMRequestProcessing:
|
||||
raise
|
||||
finally:
|
||||
# Enqueue deferred logging after post-call guardrails have written
|
||||
# guardrail_information to metadata. The finally block ensures
|
||||
# logging fires even if a guardrail raises.
|
||||
# guardrail_information to metadata.
|
||||
# For streaming early-returns: no closure is stored (wrapper_async
|
||||
# returns before the deferred block), so _enqueue_fn is None — no-op.
|
||||
# On exception (e.g. a post-call guardrail blocks the response),
|
||||
# skip firing the success closure — the exception propagates to
|
||||
# post_call_failure_hook which writes its own failure spend log.
|
||||
# Firing both produced a duplicate (Success + Failure) entry per
|
||||
# request, with the Success row exposing the blocked LLM response.
|
||||
_enqueue_fn = getattr(logging_obj, "_enqueue_deferred_logging", None)
|
||||
if _enqueue_fn is not None:
|
||||
logging_obj._enqueue_deferred_logging = None # type: ignore[union-attr]
|
||||
try:
|
||||
_enqueue_fn()
|
||||
except Exception as e:
|
||||
verbose_proxy_logger.exception(
|
||||
"Error firing deferred logging: %s", e
|
||||
)
|
||||
if not _exception_raised:
|
||||
try:
|
||||
_enqueue_fn()
|
||||
except Exception as e:
|
||||
verbose_proxy_logger.exception(
|
||||
"Error firing deferred logging: %s", e
|
||||
)
|
||||
|
||||
# Streaming cleanup: if an exception occurred AND the deferred
|
||||
# streaming closure is still set, no streaming route will
|
||||
|
||||
@@ -225,15 +225,18 @@ async def test_no_flag_fires_create_task_normally():
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 4. Non-streaming: deferred logging fires even if guardrail raises
|
||||
# 4. Non-streaming: deferred success log is suppressed when guardrail raises
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_deferred_logging_fires_on_guardrail_exception():
|
||||
async def test_deferred_logging_suppressed_on_guardrail_exception():
|
||||
"""
|
||||
If post_call_success_hook raises (e.g., guardrail blocks content),
|
||||
the deferred logging closure must still fire (via try/finally).
|
||||
If post_call_success_hook raises (e.g. a post-call guardrail blocks the
|
||||
response), the deferred async-success closure must NOT fire — the proxy's
|
||||
error path runs post_call_failure_hook which writes its own failure spend
|
||||
log. Firing both produced a duplicate (Success + Failure) entry per request,
|
||||
with the Success row exposing the blocked LLM response.
|
||||
"""
|
||||
from fastapi import HTTPException # noqa: local import for test isolation
|
||||
|
||||
@@ -264,21 +267,32 @@ async def test_deferred_logging_fires_on_guardrail_exception():
|
||||
with patch("litellm.callbacks", [guardrail]):
|
||||
proxy_logging = ProxyLogging(user_api_key_cache=DualCache())
|
||||
|
||||
# Mirrors the proxy's try / except / finally around post_call_success_hook
|
||||
# in common_request_processing.py — _exception_raised gates the closure.
|
||||
_exception_raised = False
|
||||
with pytest.raises(HTTPException):
|
||||
try:
|
||||
await proxy_logging.post_call_success_hook(
|
||||
data={"model": "gpt-4", "metadata": {}},
|
||||
response=MagicMock(),
|
||||
user_api_key_dict=UserAPIKeyAuth(api_key="test"),
|
||||
)
|
||||
try:
|
||||
await proxy_logging.post_call_success_hook(
|
||||
data={"model": "gpt-4", "metadata": {}},
|
||||
response=MagicMock(),
|
||||
user_api_key_dict=UserAPIKeyAuth(api_key="test"),
|
||||
)
|
||||
except Exception:
|
||||
_exception_raised = True
|
||||
raise
|
||||
finally:
|
||||
# Mirrors the proxy's finally block
|
||||
_enqueue_fn = getattr(logging_obj, "_enqueue_deferred_logging", None)
|
||||
if _enqueue_fn is not None:
|
||||
logging_obj._enqueue_deferred_logging = None
|
||||
_enqueue_fn()
|
||||
if not _exception_raised:
|
||||
_enqueue_fn()
|
||||
|
||||
assert enqueue_called is True
|
||||
assert _exception_raised is True
|
||||
assert enqueue_called is False, (
|
||||
"Deferred success log must not fire when the post-call guardrail "
|
||||
"raised — post_call_failure_hook writes its own failure log."
|
||||
)
|
||||
assert logging_obj._enqueue_deferred_logging is None
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user