fix(proxy): capture logging_obj before post_call_failure_hook pops it in ModifyResponseException streaming path - #32651
Closed
mateo-berri wants to merge 2 commits into
Conversation
… in ModifyResponseException streaming path post_call_failure_hook removes litellm_logging_obj from request_data before iterating callbacks (it's not serialisable). The streaming branch of the ModifyResponseException handler read it from _data after that call, so it always received None and CustomStreamWrapper.__init__ crashed with AttributeError: NoneType has no attribute model_call_details. Capture it before the hook runs so the streaming path gets a valid object. Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
…ging_obj capture Covers the bug where logging_obj was read from request_data after post_call_failure_hook had already popped it, causing CustomStreamWrapper to crash with AttributeError. Co-authored-by: Mateo Wang <mateo-berri@users.noreply.github.com>
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
|
Superseded by #32665, which contains the exact same change under my authorship. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relevant issues
Found during live smoke testing of PR #32289 (LIT-4186 Bedrock
disable_exception_on_blockfix).Linear ticket
Pre-Submission checklist
Screenshots / Proof of Fix
Live proxy on
:4000against real AWS Bedrock guardrailwymft0xktn2m(blocks admin-related prompts with message "Sorry, the model cannot answer this question.").Before the fix: streaming pre_call block returns HTTP 500
Root cause:
post_call_failure_hookpopslitellm_logging_objfromrequest_databefore invoking callbacks (comment: "Remove before callbacks iterate — not serialisable"). The streaming branch ofModifyResponseExceptionhandler inchat_completionreadlogging_objfrom_dataafter that call, always receivingNone.CustomStreamWrapper.__init__then crashed:AttributeError: 'NoneType' object has no attribute 'model_call_details'.After the fix: streaming pre_call block returns HTTP 200 with valid SSE
All other cases confirmed working against live Bedrock:
finish_reason=content_filter, zero usageType
🐛 Bug Fix
Changes
litellm/proxy/proxy_server.py: capturelogging_objfrom_databefore callingpost_call_failure_hook(which pops it). Pass the captured value toCustomStreamWrapperrather than re-reading from the dict.tests/test_litellm/proxy/guardrails/guardrail_hooks/test_bedrock_guardrails.py: addtest_chat_completion_modify_response_exception_streaming_logging_obj_not_noneto lock in the fix and prevent regression.