-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Set ToolRetryError message
#3718
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
b41f23b
916a8e1
df91f43
993939a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -187,14 +187,14 @@ async def _call_tool( | |
| content=e.errors(include_url=False, include_context=False), | ||
| tool_call_id=call.tool_call_id, | ||
| ) | ||
| e = ToolRetryError(m) | ||
| e = ToolRetryError(m, message=str(e)) | ||
| elif isinstance(e, ModelRetry): | ||
| m = _messages.RetryPromptPart( | ||
| tool_name=name, | ||
| content=e.message, | ||
| tool_call_id=call.tool_call_id, | ||
| ) | ||
| e = ToolRetryError(m) | ||
| e = ToolRetryError(m, message=e.message) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wouldn't mind setting
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to support an automatic behavior for
Which do you prefer?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, since my idea below of getting it from |
||
| else: | ||
| assert_never(e) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,10 +13,12 @@ | |
| IncompleteToolCall, | ||
| ModelAPIError, | ||
| ModelHTTPError, | ||
| ToolRetryError, | ||
| UnexpectedModelBehavior, | ||
| UsageLimitExceeded, | ||
| UserError, | ||
| ) | ||
| from pydantic_ai.messages import RetryPromptPart | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
|
|
@@ -32,6 +34,7 @@ | |
| lambda: ModelAPIError('model', 'test message'), | ||
| lambda: ModelHTTPError(500, 'model'), | ||
| lambda: IncompleteToolCall('test'), | ||
| lambda: ToolRetryError(RetryPromptPart(content='test', tool_name='test')), | ||
| ], | ||
| ids=[ | ||
| 'ModelRetry', | ||
|
|
@@ -44,6 +47,7 @@ | |
| 'ModelAPIError', | ||
| 'ModelHTTPError', | ||
| 'IncompleteToolCall', | ||
| 'ToolRetryError', | ||
| ], | ||
| ) | ||
| def test_exceptions_hashable(exc_factory: Callable[[], Any]): | ||
|
|
@@ -59,3 +63,10 @@ def test_exceptions_hashable(exc_factory: Callable[[], Any]): | |
|
|
||
| assert exc in s | ||
| assert d[exc] == 'value' | ||
|
|
||
|
|
||
| def test_tool_retry_error_str(): | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should test that it is set correctly in the
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good! |
||
| """Test that ToolRetryError uses provided message.""" | ||
| part = RetryPromptPart(content='some content', tool_name='my_tool') | ||
| error = ToolRetryError(part, message='custom message') | ||
| assert str(error) == 'custom message' | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
ToolRetryError.__init__is able to access the__cause__that's set byfrom e(I'm not sure if it can), we could do this automaticallyThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked locally. Unfortunately,
__cause__isNonein__init__. It gets set after the exception is constructed, by theraise ... from esyntax