fix(logging): add async_release_disconnect_state_hook as a default no-op on CustomLogger

Every other optional hook on CustomLogger ships as an empty method a subclass
can override; this one didn't, so _release_disconnect_state_on_all_callbacks
calling it on any callback that doesn't implement it (nearly all of them)
raised AttributeError, caught and debug-logged on every single disconnect.
bugbot caught this on review.
This commit is contained in:
Deepanshu 2026-08-26 14:01:43 -04:00
parent 3a2f893134
commit 54a7f6f7b3
2 changed files with 30 additions and 0 deletions

View file

@ -187,6 +187,16 @@ class CustomLogger: # https://docs.litellm.ai/docs/observability/custom_callbac
async def async_log_failure_event(self, kwargs, response_obj, start_time, end_time):
pass
async def async_release_disconnect_state_hook(self, request_data: Mapping[str, object]) -> None:
"""
Called when a client disconnects mid-request, for a callback that reserved
per-request state outside async_log_success_event/async_log_failure_event
(e.g. a concurrency slot admitted before the first response chunk) -- those
two callbacks never run for a client disconnect, so a callback relying on
them alone to release such state would otherwise leak it until its own
safety-net TTL.
"""
async def async_log_audit_log_event(self, audit_log: "StandardAuditLogPayload"):
"""Called when an audit log is created. Override in subclasses to handle."""

View file

@ -33,6 +33,7 @@ from litellm.proxy.common_request_processing import (
ttft_keepalive_interval,
_override_openai_response_model,
_parse_event_data_for_error,
_release_disconnect_state_on_all_callbacks,
_resolve_per_request_model_group_alias,
_should_return_raw_model_name,
_UpstreamClosingStreamingResponse,
@ -4091,6 +4092,25 @@ class TestCancelOnDisconnect:
assert exc_info.value.status_code == 499
assert recorder.disconnect_hook_calls == 1
async def test_release_disconnect_state_calls_every_callback_including_ones_without_an_override(
self, monkeypatch
):
"""
Bugbot finding: CustomLogger never defined async_release_disconnect_state_hook
as an empty default, unlike every other optional hook on that base class, so
calling it on a callback that never overrides it (most registered callbacks)
raised AttributeError -- caught here, but still a real gap in the base class's
own contract that a genuine implementation bug would be indistinguishable from.
"""
overriding = _RecordingDisconnectHookLogger()
bare = CustomLogger()
monkeypatch.setattr(litellm, "callbacks", [overriding, bare])
await _release_disconnect_state_on_all_callbacks({"litellm_call_id": "call-1"})
assert overriding.disconnect_hook_calls == 1
assert await bare.async_release_disconnect_state_hook({"litellm_call_id": "call-1"}) is None
async def _drive_base_process_llm_request(
self, monkeypatch, general_settings: dict, llm_call, request: Request
):