fix(api): use .get() instead of .pop() for request in exception inter… - #6229
fix(api): use .get() instead of .pop() for request in exception inter…#6229shafeeq27edu-ai wants to merge 2 commits into
Conversation
…ceptor Prevents destructive mutation of the caller's kwargs dict. If the intercepted function is retried, the request key remains available.
|
@shafeeq27edu-ai is attempting to deploy a commit to the agenta projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughSummary by CodeRabbit
Walkthrough
ChangesException interception
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This localized fix preserves the request data for retries and downstream handling without changing intended endpoint behavior; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Description
This PR fixes a destructive mutation bug in the exception interceptor utility.
kwargs.pop("request")was silently removing therequestkey from the caller's dictionary, which could cause missing-key errors on retries or downstream usage.Summary
What changed? Changed
kwargs.pop("request", None)tokwargs.get("request", None)inapi/oss/src/utils/exceptions.py.Why was this change needed?
pop()mutates the originalkwargsdict. If the intercepted function is retried or the kwargs are reused, the"request"key is missing. This is especially dangerous because the code is inside asuppress()context manager that swallows exceptions silently.What problem does it solve? Prevents subtle missing-key bugs in every API endpoint that uses the exception interceptor.
How the change addresses the root cause: By using
.get()instead of.pop(), we perform a non-destructive read. The caller'skwargsdict remains intact.Testing
Verified locally
exceptions.pyto confirmrequestis not required to be removed fromkwargsfor downstream logicruff check api/oss/src/utils/exceptions.py— passes with no errorsruff format api/oss/src/utils/exceptions.py— no changes neededAdded or updated tests
N/A — This is a 1-line safety fix with no behavior change. Existing API tests cover the interceptor path.
QA follow-up
N/A — The fix is self-contained and verified by static analysis.
Demo
BEFORE FIX :
AFTER FIX:
Checklist