fix(proxy): exclude proxy_server_request from its own body snapshot#28618
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes a self-referencing loop in the
Confidence Score: 5/5Safe to merge — the change is a minimal, well-scoped fix with a direct regression test and no observable side effects on correct code paths. The fix is a single dict-comprehension filter change that removes a pathological self-reference from the body snapshot. The updated comment clearly explains both exclusions. The new regression test follows the established pattern in the file, uses only mocks, and directly asserts the invariant that was previously broken. No existing tests are modified, and the behavioral change only removes a key that was producing broken (self-referential) output anyway. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/litellm_pre_call_utils.py | One-line behavioral fix: adds proxy_server_request to the body snapshot exclude set to prevent the dict from self-referencing and causing infinite traversal loops in downstream consumers. |
| tests/test_litellm/proxy/test_litellm_pre_call_utils.py | Adds a new regression test mirroring the existing _excludes_secret_fields test; uses mocks only with no real network calls, consistent with the test directory rules. |
Reviews (1): Last reviewed commit: "fix(proxy): exclude proxy_server_request..." | Re-trigger Greptile
9c0a98c
into
litellm_internal_staging
Relevant issues
n/a — internal bug found while investigating the redaction-gaps work in PR #28611. Independent file/owner, shipping separately.
Linear ticket
n/a
Pre-Submission checklist
tests/test_litellm/directory — addedtest_add_litellm_data_to_request_body_snapshot_excludes_proxy_server_requestintests/test_litellm/proxy/test_litellm_pre_call_utils.py, mirroring the existing_excludes_secret_fieldsregression test.make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
n/a
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
Before —
add_litellm_data_to_requestwrites the body snapshot at the end of pre-call processing:At that point
data["proxy_server_request"]was already set earlier in the same function, so the snapshot includes it andbodyself-references:Any consumer that walks
proxy_server_request.body(custom loggers, spend-tracking, audit exporters) hits an infinite loop. Reproduced on a test proxy by inspecting the kwargs payload passed to aCustomLogger.async_log_success_eventhook — the body dict's KEYS includeproxy_server_requeston every successful call, regardless of redaction.After —
proxy_server_requestis added to the snapshot exclude set alongsidesecret_fields. Same repro proxy, same hook, body's KEYS no longer includeproxy_server_requeston any of three test requests (baseline / redaction header non-reasoning / redaction header reasoning). New regression test covers it.(Pre-fix this read
2×— one legitimate occurrence atlitellm_params.proxy_server_request, one in the self-referenced snapshot.)Type
🐛 Bug Fix
Changes
litellm/proxy/litellm_pre_call_utils.py— one-line behavior change inadd_litellm_data_to_request. The body snapshot exclude set goes from{\"secret_fields\"}to{\"secret_fields\", \"proxy_server_request\"}:Comment block above the snapshot updated to explain both exclusions (was: secret_fields only; now: secret_fields + the self-reference explanation for proxy_server_request).
tests/test_litellm/proxy/test_litellm_pre_call_utils.py— addedtest_add_litellm_data_to_request_body_snapshot_excludes_proxy_server_request. Asserts"proxy_server_request" not in updated["proxy_server_request"]["body"].Known follow-up, intentionally out of scope: the body snapshot also drags in other pipeline-internal keys (
litellm_call_id,litellm_trace_id,user_api_key_*,api_version,ttl) that were never part of the user POST body. Broadening the exclude set risks breaking downstream consumers that read those keys today — flagging as a follow-up rather than bundling here.