Skip to content
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

To support configuring how errors are handled in tools. #5274

Open
ekzhu opened this issue Jan 30, 2025 · 6 comments · May be fixed by #5488
Open

To support configuring how errors are handled in tools. #5274

ekzhu opened this issue Jan 30, 2025 · 6 comments · May be fixed by #5488

Comments

@ekzhu
Copy link
Collaborator

ekzhu commented Jan 30, 2025

Should be part of the configuration parameter for FunctionTool.

@ekzhu ekzhu added this to the python-v0.4.x milestone Jan 30, 2025
@wistuba
Copy link
Contributor

wistuba commented Feb 10, 2025

How do you imagine this to work? Like this?

caught_errors: Sequence[Exception] | Exception
stock_price_tool = FunctionTool(
    get_stock_price,
    description="Fetch the stock price for a given ticker.",
    caught_errors=caught_errors
)

where caught_errors defaults to Exception. Any error not in caught_errors is raised.

Better name for caught_errors would be good.

@ekzhu
Copy link
Collaborator Author

ekzhu commented Feb 11, 2025

I think what you proposed is good. Perhaps name it "return_errors", as the error messages are being returned.

@wistuba
Copy link
Contributor

wistuba commented Feb 11, 2025

Quick follow-up: this is not specific for the FunctionTool, right? This will change the Tool protocol and add a new property returned_errors, right? The logic dealing with exceptions, e.g., AssistantAgent._execute_tool_call assumes a BaseTool, not a FunctionTool.

@ekzhu
Copy link
Collaborator Author

ekzhu commented Feb 11, 2025

Quick follow-up: this is not specific for the FunctionTool, right? This will change the Tool protocol and add a new property returned_errors, right? The logic dealing with exceptions, e.g., AssistantAgent._execute_tool_call assumes a BaseTool, not a FunctionTool.

Yes if it is a boolean field as I think the returned value itself is the error message.

Though I think the configuration should be on FunctionTool. it should be on the implementation of BaseTool to correctly specify the returned_errors.

@ekzhu
Copy link
Collaborator Author

ekzhu commented Feb 11, 2025

It does seem like a breaking change... Is it possible to bypass BaseTool for now? Having the error raised directly from BaseTool, and use the is_error field in FunctionExecutionResult to inform the model of the error?

@ekzhu
Copy link
Collaborator Author

ekzhu commented Feb 12, 2025

Ah.. Okay appologies for the confusion. I misunderstood what you meant by returned_errors -- I thought it meant the errors are returned as part of the ReturnT of BaseTool.

Please ignore my two comments above.

It makes sense. Let's move the conversation to the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants