From 54a7f6f7b319ccfb10a2fea480175c76362831da Mon Sep 17 00:00:00 2001 From: Deepanshu Date: Wed, 26 Aug 2026 14:01:43 -0400 Subject: [PATCH] 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. --- litellm/integrations/custom_logger.py | 10 ++++++++++ .../proxy/test_common_request_processing.py | 20 +++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/litellm/integrations/custom_logger.py b/litellm/integrations/custom_logger.py index 41caf732db0..139fc2b2e65 100644 --- a/litellm/integrations/custom_logger.py +++ b/litellm/integrations/custom_logger.py @@ -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.""" diff --git a/tests/test_litellm/proxy/test_common_request_processing.py b/tests/test_litellm/proxy/test_common_request_processing.py index f854d8f94e5..e092a6d6272 100644 --- a/tests/test_litellm/proxy/test_common_request_processing.py +++ b/tests/test_litellm/proxy/test_common_request_processing.py @@ -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 ):