fix(tools): raise ToolUsageError instead of bare raise on non-dict args#6486
fix(tools): raise ToolUsageError instead of bare raise on non-dict args#6486HumphreySun98 wants to merge 1 commit into
Conversation
_original_tool_calling had a bare `raise` on the `not isinstance(arguments, dict)` path, which is outside the preceding try/except — so with raise_error=True it fails with "RuntimeError: No active exception to re-raise" instead of surfacing the intended tool-arguments error. Raise the ToolUsageError explicitly. Fixes crewAIInc#6430 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughFixed a bare ChangesBare raise fix and tests
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
ErenAta16
left a comment
There was a problem hiding this comment.
Reproduced the underlying bug standalone: a bare raise outside an except block raises RuntimeError: No active exception to reraise instead of the intended ToolUsageError, so callers relying on except ToolUsageError around _original_tool_calling would get the wrong exception type entirely when raise_error=True and the arguments are not a dict.
This is one of five open PRs fixing the exact same line (#6512, #6511, #6499, #6495, and this one) — all correct, all equivalent in behavior. This version is the cleanest of the five: it builds the ToolUsageError once and reuses it for both the raise and the return branches (the other four construct it twice, once per branch, which is harmless but redundant), and the added comment explains why the bare raise fails here specifically, which is useful context for whoever reviews this cluster. Worth picking one of the five and closing the rest as duplicates — LGTM on this one as the pick.
Problem
ToolUsage._original_tool_callinghas a bareraiseon the non-dict path, which sits outside the precedingtry/except:_tool_calling()calls this withraise_error=True, so if_validate_tool_inputever returns a non-dict, the bareraisefails withRuntimeError: No active exception to re-raise— masking the intended tool-arguments error. It's a latent landmine today (that helper currently always returns a dict or raises), but it's still incorrect.Fix
Raise the intended
ToolUsageErrorexplicitly:The bare
raiseat the earlier site (insideexcept) is correct and left unchanged.Tests
Added two tests in
test_tool_usage.py(mock_validate_tool_input→ non-dict):raise_error=TrueraisesToolUsageError(notRuntimeError), andraise_error=Falsereturns aToolUsageError. Verified failing-without/passing-with the fix; fulltest_tool_usage.py(30) green;ruff+mypyclean.Fixes #6430
This PR was authored with Claude Code. Per
CONTRIBUTING.md, AI-generated contributions require thellm-generatedlabel — I don't have triage permission to set it, so could a maintainer please add it? 🤖 Generated with Claude Code