fix(proxy): use e.request_data for logging_obj in ModifyResponseException streaming passthrough - #30800
Conversation
…tion streaming passthrough
When a guardrail blocks a streaming request pre-call by raising
ModifyResponseException (or RejectedRequestError), chat_completion streams the
violation message back as a 200 by building a CustomStreamWrapper. It read the
logging object from the outer request body (`data.get("litellm_logging_obj")`),
but that dict never carries litellm_logging_obj -- it diverges from the
processor's data at function_setup, and only the processor copy (exposed as
e.request_data, already bound to `_data` here) gets the logging object
attached. CustomStreamWrapper.__init__ then dereferences
`logging_obj.model_call_details` on None and 500s the request with
"AttributeError: 'NoneType' object has no attribute 'model_call_details'".
Read logging_obj from `_data` (= e.request_data) in both streaming
passthrough handlers so the refusal streams correctly. The non-streaming and
the anthropic/responses passthrough paths were unaffected.
Adds a regression test asserting the wrapper receives the logging object from
e.request_data rather than None.
The streaming logging_obj fix was applied to both the ModifyResponseException and RejectedRequestError handlers, but only the former had a regression test. Extract a shared helper and add a parallel test for the RejectedRequestError streaming path so both handlers stay guarded against the None-logging_obj crash.
|
Generated by Claude Code |
Greptile SummaryFixes a 500 error triggered when a guardrail blocks a streaming chat-completion request pre-call via
Confidence Score: 5/5Safe to merge — two one-line changes in isolated exception handlers with targeted regression tests covering both paths. The change is a two-line fix in well-understood exception handlers that only activate when a guardrail raises during streaming; the non-streaming path and all other routes are untouched. The added tests directly exercise the broken path and verify the fix without making real network calls. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/proxy_server.py | Two one-line fixes: both streaming passthrough handlers (ModifyResponseException and RejectedRequestError) now read litellm_logging_obj from _data (e.request_data) instead of the outer request body data, which never carries that key. |
| tests/test_litellm/proxy/test_modify_response_streaming_passthrough.py | New regression test file using mocks only; exercises both ModifyResponseException and RejectedRequestError streaming passthrough paths and asserts CustomStreamWrapper receives the logging object from e.request_data rather than None. |
Reviews (2): Last reviewed commit: "test(proxy): cover RejectedRequestError ..." | Re-trigger Greptile
|
bugbot run Generated by Claude Code |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit dd7db9a. Configure here.
…tion streaming passthrough (BerriAI#30800) * fix(proxy): use e.request_data for logging_obj in ModifyResponseException streaming passthrough When a guardrail blocks a streaming request pre-call by raising ModifyResponseException (or RejectedRequestError), chat_completion streams the violation message back as a 200 by building a CustomStreamWrapper. It read the logging object from the outer request body (`data.get("litellm_logging_obj")`), but that dict never carries litellm_logging_obj -- it diverges from the processor's data at function_setup, and only the processor copy (exposed as e.request_data, already bound to `_data` here) gets the logging object attached. CustomStreamWrapper.__init__ then dereferences `logging_obj.model_call_details` on None and 500s the request with "AttributeError: 'NoneType' object has no attribute 'model_call_details'". Read logging_obj from `_data` (= e.request_data) in both streaming passthrough handlers so the refusal streams correctly. The non-streaming and the anthropic/responses passthrough paths were unaffected. Adds a regression test asserting the wrapper receives the logging object from e.request_data rather than None. * test(proxy): cover RejectedRequestError streaming passthrough The streaming logging_obj fix was applied to both the ModifyResponseException and RejectedRequestError handlers, but only the former had a regression test. Extract a shared helper and add a parallel test for the RejectedRequestError streaming path so both handlers stay guarded against the None-logging_obj crash. --------- Co-authored-by: Joseph Barker <joseph.barker@rubrik.com>
Relevant issues
Internal copy of #30701 by @seph-barker (Joseph Barker) so we can run it through CircleCI. Full credit for the fix and tests goes to the original author; the two commits here are cherry-picked from that PR with authorship preserved.
A guardrail that blocks a streaming request pre-call (by raising
ModifyResponseExceptionorRejectedRequestError) 500s the request instead of streaming the violation message back as a 200.Pre-Submission checklist
Type
🐛 Bug Fix
Changes
The bug
When a guardrail blocks a streaming request pre-call,
chat_completion'sexcept ModifyResponseException/except RejectedRequestErrorhandlers stream the violation message back as a 200 by building aCustomStreamWrapper:datahere is the outer request body returned by_read_request_body. It never carrieslitellm_logging_obj: the processor's data dict diverges from the outer body atfunction_setup(which returns a fresh kwargs dict), andlitellm_logging_objis attached only to that processor copy. Sodata.get("litellm_logging_obj")isNone, andCustomStreamWrapper.__init__immediately dereferences it:The non-streaming path is unaffected (it returns a
ModelResponsedirectly), and the anthropic / responses-API passthrough paths are unaffected (they stream via a plain SSE generator, notCustomStreamWrapper). Only streaming + pre-call block hits this.The fix
_data = e.request_datais already bound two lines above and is the processor's data dict, which does carrylitellm_logging_obj. Readlogging_objfrom_datain both streaming passthrough handlers (ModifyResponseExceptionandRejectedRequestError).Added a regression test (
tests/test_litellm/proxy/test_modify_response_streaming_passthrough.py) that driveschat_completion'sModifyResponseExceptionstreaming branch with an outer body lackinglitellm_logging_objand asserts theCustomStreamWrapperreceives the logging object frome.request_data(it would beNonewithout the fix).Generated by Claude Code
Note
Low Risk
Narrow bug fix in guardrail streaming error handlers with targeted regression tests; no auth or data-model changes.
Overview
Fixes 500 errors when a guardrail blocks a streaming chat completion pre-call via
ModifyResponseExceptionorRejectedRequestError, where the intended behavior is to stream the violation as a 200 throughCustomStreamWrapper.In both streaming passthrough branches in
chat_completion,logging_objis now taken from_data(e.request_data) instead of the outerdatafrom_read_request_body. Only the processor copy carrieslitellm_logging_obj; using the outer body leftlogging_objasNoneand triggeredAttributeErrorinsideCustomStreamWrapper.Adds regression tests that assert
CustomStreamWrapperreceives the logging object from the exception’srequest_datawhen the outer body lacks it.Reviewed by Cursor Bugbot for commit dd7db9a. Bugbot is set up for automated code reviews on this repo. Configure here.