fix(guardrails/bedrock): honor disable_exception_on_block by raising ModifyResponseException - #32289
Conversation
…ModifyResponseException The Bedrock-specific GuardrailInterventionNormalStringError predates the unified guardrails refactor and no proxy code path handles it, so a block with the flag set surfaced as an uncaught Exception -> HTTP 500 in pre_call mode and was silently discarded in during_call mode (model call proceeded in the parallel asyncio.gather; the block hook's data["mock_response"] mutation happened after route_request had already unpacked kwargs). Convert the block to ModifyResponseException at the raise site inside make_bedrock_api_request. That exception is the industry-standard proxy contract already caught in proxy_server, anthropic_endpoints, response_api _endpoints, and pass_through_endpoints; it turns into a 200 response with finish_reason=content_filter and the block message as content, which is exactly what the flag was documented to yield. Post-call blocks attach the LLM response to original_response so the synthetic reply reports the upstream call's real token usage instead of zero. Deletes the now-orphaned GuardrailInterventionNormalStringError class and the dead create_guardrail_blocked_response / mock_response plumbing in the Bedrock hooks; updates the existing tests that had locked in the buggy contract. Resolves LIT-4186
Greptile SummaryThis PR fixes two bugs in the Bedrock guardrail when
Confidence Score: 5/5Safe to merge; the changes are well-scoped to the Bedrock guardrail hooks and align with the existing ModifyResponseException contract used by all other endpoint handlers. The fix is mechanically straightforward — a single raise-site conversion backed by thorough regression tests. Removed code has no surviving callers. The streaming post_call case is correctly handled in-band. Type narrowing for No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/exceptions.py | Removes GuardrailInterventionNormalStringError; no remaining imports of the class anywhere in the codebase. |
| litellm/proxy/guardrails/guardrail_hooks/bedrock_guardrails.py | Core fix: _get_http_exception_for_blocked_guardrail now returns ModifyResponseException instead of the orphaned GuardrailInterventionNormalStringError; pre/during_call hooks let the exception propagate; post_call attaches original LLM response for usage preservation; streaming post_call catches and converts to synthetic stream; dead create_guardrail_blocked_response and data["mock_response"] paths removed. |
| tests/enterprise/litellm_enterprise/proxy/guardrails/test_bedrock_apply_guardrail.py | Regression test updated from GuardrailInterventionNormalStringError to ModifyResponseException; assertion now directly accesses .message attribute instead of str(exc_info.value.message). |
| tests/guardrails_tests/test_bedrock_guardrails.py | Two tests that locked in the buggy contract (no exception raised, mock_response set) updated to assert correct behavior (ModifyResponseException raised; streaming delivers synthetic content_filter chunks). |
| tests/test_litellm/proxy/guardrails/guardrail_hooks/test_bedrock_guardrails.py | Six new regression tests added covering all three hook types (pre_call, during_call, post_call), the unified apply_guardrail path, streaming synthetic chunk shape, and streaming usage preservation. |
Reviews (4): Last reviewed commit: "fix(guardrails/bedrock): preserve upstre..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
bugbot run |
…ith_updated_bedrock_guardrail_response Follow-up to the disable_exception_on_block fix. That method used to receive either a BedrockGuardrailResponse or a plain string (the block message, when the flag was set). Now that a block always raises ModifyResponseException before this method runs, the string branch is unreachable; tighten the type to BedrockGuardrailResponse and delete the guard.
|
Fixed in 483d34c: dropped the unreachable str branch and tightened the type to BedrockGuardrailResponse. On the Cursor Bugbot finding: streaming post_call blocks are out of scope for this PR (the ticket covers pre_call and during_call). Pre-fix, streaming post_call silently discarded blocks; post-fix, it surfaces as a 500 via the streaming error path. Both wrong; the fix is teaching the bedrock streaming hook to emit a synthetic block through build_block_sse_chunks the way unified_guardrail does. Filing a follow-up. |
…tream instead of surfacing as SSE 500 Regression from the LIT-4186 refactor: pre-refactor, the streaming post_call iterator caught GuardrailInterventionNormalStringError locally and replaced the assembled response with a synthetic content-filter message, then re-emitted it as chunks via MockResponseIterator. After the refactor the exception was re-raised as ModifyResponseException, which async_streaming_data_generator serializes as a proxy 500 error frame because the SSE response headers are already flushed by the time the block fires. Non-streaming paths still let ModifyResponseException propagate to the endpoint handler (which converts it into a 200). Streaming can't do that, so keep the local synthesis: on the exception, rebind the assembled response to a ModelResponse whose single choice carries the block message as content and finish_reason=content_filter, and let the downstream MockResponseIterator emit it as chunks. Same shape a non-streaming block produces. Adds a mapped-file regression test that mutation-kills the raise behavior and locks in the synthetic-stream contract.
|
bugbot run |
…ll block Non-streaming post_call blocks report the upstream LLM call's real token usage via ModifyResponseException.original_response, which the endpoint handler unwraps through _blocked_response_usage. Streaming post_call synthesizes its own ModelResponse locally (the exception can't escape the SSE generator), and previously left .usage unset, so the client saw accurate billing on non-streaming blocks and zero on streaming blocks -- silent revenue leak. Copy the assembled response's .usage onto the synthetic block response before yielding. Pre-refactor code had the same gap (create_guardrail_blocked_response never set usage); this is a net improvement, not a regression fix.
|
Valid finding, fixed in b9ce36f. Streaming block now copies .usage from the assembled response to the synthetic block ModelResponse before yielding, so streaming and non-streaming blocks report equivalent token usage. Note this is a net improvement over pre-refactor behavior (the old create_guardrail_blocked_response never set usage either), not a regression fix. |
|
bugbot run |
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 b9ce36f. Configure here.
Relevant issues
Linear ticket
Resolves LIT-4186
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Live proxy on
:4000against a real AWS Bedrock guardrail. The guardrail has one topic-policy denial on the word "coffee" withblockedInputMessagingset toSorry, the model cannot answer this question.Launched with:
Config declares four guardrails against the same Bedrock identifier:
bedrock-guard-pre-call(modepre_call,disable_exception_on_block: true),bedrock-guard-during-call(during_call,disable_exception_on_block: true),bedrock-guard-post-call(post_call,disable_exception_on_block: true), andbedrock-guard-pre-call-strict(pre_call,disable_exception_on_block: false) for the negative regression check.Before the fix:
pre_callreturns HTTP 500 with the block messageBefore the fix:
during_callsilently returns the real model response688 completion tokens billed for a request the guardrail intervened on
After the fix:
pre_callreturns HTTP 200 with the block message as contentAfter the fix:
during_callalso returns HTTP 200; zero usage, no billable LLM callAfter the fix:
post_callnon-streaming block reports upstream call's real usageThe guardrail also blocks output containing "coffee". Ask a question whose answer will contain it so the LLM runs first and the guardrail then blocks the output; the synthetic block reply carries the upstream call's real token usage instead of zero.
31 completion tokens from the upstream Bedrock call preserved on the block response.
After the fix:
post_callstreaming block emits a clean SSE synthetic stream with real usageSame request with
"stream": trueandstream_options.include_usage: true. SSE headers are already flushed by the time the block fires, so the streaming iterator can't rely on the endpoint handler's 200 conversion; it synthesizes the block chunks locally and copies the upstream call's usage onto them.Real upstream usage on the streaming block,
finish_reason=content_filter, clean[DONE]terminator; no error frame.Regression check: flag=false blocked request still returns HTTP 400
Regression check: allowed input still hits the real model
Regression check: allowed input, streaming, still hits the real model and reports usage
Type
🐛 Bug Fix
Changes
BedrockGuardrailused to raise a Bedrock-specificGuardrailInterventionNormalStringErrorwhen a block coincided withdisable_exception_on_block: true. That exception predates the unified guardrails refactor and no proxy code path handles it. Inpre_callmode it escaped the unified path as an uncaughtException, surfacing as HTTP 500 whose body carried the guardrail'sblockedInputMessaging, which end users read as the model itself refusing. Induring_callmode the native hook did catch the exception and setdata["mock_response"], butroute_requestincommon_request_processing.pyunpacks kwargs before the moderation task in the parallelasyncio.gathergets a chance to run, so the mutation was dead code and the real model response won.The fix converts a block with the flag set into
ModifyResponseExceptionat the raise site insidemake_bedrock_api_request. That exception is already the standard proxy contract for guardrails that want a 200 response with a synthetic content-filter reply; it is caught inproxy_server.py,anthropic_endpoints/endpoints.py,response_api_endpoints/endpoints.py, andpass_through_endpoints.py.during_callblocks now cancel the parallel LLM task via the existing_cancel_pending_gather_taskspath, so no billable model call happens. Non-streaming post-call blocks attach the LLM response toModifyResponseException.original_responseso the synthetic reply reports the upstream call's real token usage rather than discarding it.Streaming post_call needs a different shape. The SSE response headers are already flushed by the time the block fires, so a raise from the streaming iterator would be serialized by
async_streaming_data_generatoras a proxy error frame with code 500. Instead the streaming hook catchesModifyResponseExceptionlocally, snapshots the assembled response's.usage, rebinds the assembled response to a syntheticModelResponsewhose single choice carries the block message withfinish_reason=content_filter, copies the snapshotted usage onto it, and lets the existingMockResponseIteratoremit it as chunks. That produces the same shape a non-streaming block produces on the same guardrail.The now-orphaned
GuardrailInterventionNormalStringErrorclass and the deadcreate_guardrail_blocked_response/data["mock_response"]plumbing in the Bedrock hooks are removed. Two pre-existing tests that had locked in the buggy contract are updated to assert the correct behavior. The mapped test filetests/test_litellm/proxy/guardrails/guardrail_hooks/test_bedrock_guardrails.pygains eight focused regression tests covering the raise-site conversion, all three hook types (pre_call,during_call,post_call), the unifiedapply_guardrailpath, the streaming post_call synthetic-stream contract, and streaming usage preservation; each test was mutation-checked against the unfixed source and observed to fail before passing on the fix.