fix(tools): raise ToolUsageError instead of bare raise for non-dict arguments#6495
fix(tools): raise ToolUsageError instead of bare raise for non-dict arguments#6495ashusnapx wants to merge 1 commit into
Conversation
|
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)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough
ChangesBare Raise Fix
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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.
Same one-line fix as #6486, #6512, #6511, #6499 — this is one of five open PRs (added this one to the tally) independently fixing the same bare-raise-outside-except bug in _original_tool_calling. Reproduced the bug standalone: the bare raise here raises RuntimeError: No active exception to reraise rather than the intended ToolUsageError, since it is outside any except block. This PR's fix is correct and equivalent to the others. Left a fuller comparison on #6486, which constructs the error once and reuses it for both branches rather than twice — noting the duplication here so only one lands.
…rguments Closes crewAIInc#6430 _original_tool_calling() contained a bare `raise` on the non-dict arguments path (line 853) outside any except block. When _validate_tool_input returned a non-dict and raise_error=True, this produced RuntimeError: No active exception to re-raise instead of the intended ToolUsageError. Replace with explicit `raise ToolUsageError(...)`. The except block above (line 846-848) correctly uses bare `raise` inside an active exception context — left unchanged. Also resolves ruff PLE0704 (bare raise not inside exception handler). Tests: 4 new regression tests covering the non-dict branch with both raise_error settings, the _tool_calling fallback flow, and verification that RuntimeError is never raised. All 32 tests in test_tool_usage.py pass. ruff clean.
a395ea2 to
ae7ebf9
Compare
|
Fixed — error is now constructed once and reused for both branches (raise and return), matching the approach in #6486. Thanks for the catch. |
|
Confirmed - the current diff constructs |
|
Thanks for confirming. Ready for maintainer review. |
Summary
Fixes
RuntimeError: No active exception to re-raiseinToolUsage._original_tool_calling()when_validate_tool_inputreturns a non-dict andraise_error=True.Closes #6430
Problem
_original_tool_calling()attool_usage.py:853had a bareraiseoutside anyexceptblock:When this path was hit with
raise_error=True, Python raisedRuntimeError: No active exception to re-raiseinstead of the intendedToolUsageError. The_tool_calling()fallback relies on catching a meaningful exception to trigger its retry logic, so this produced a confusing RuntimeError.Also flagged by
ruff check --select PLE0704.Fix
Replace bare
raisewith explicitraise ToolUsageError(...):The
exceptblock above (line 846-848) correctly uses bareraiseinside an active exception context — left unchanged.Tests (4 new)
test_original_tool_calling_non_dict_raises_tool_usage_errorraise_error=TrueraisesToolUsageError, notRuntimeErrortest_original_tool_calling_non_dict_returns_errorraise_error=FalsereturnsToolUsageErrortest_tool_calling_fallback_on_non_dict_tool_callingflow falls back gracefullytest_non_dict_raises_not_runtime_errorToolUsageError, notRuntimeErrorAll 32 tests in
test_tool_usage.pypass.ruff check --select PLE0704clean.Verification