feat: select errors that will be caught when raised within a tool

[wistuba]

Conversation starter to agree on the implementation. When we are aligned, I'll add tests.

Why are these changes needed?

Not every tool error should be returned to the agent. This change allows to cherry-pick which exceptions we want to share with the agent.

Related issue number

Closes: #5274

Checks

• I've included any doc changes needed for https://microsoft.github.io/autogen/. See https://microsoft.github.io/autogen/docs/Contribute#documentation to build and test documentation locally.
• I've added tests (if relevant) corresponding to the changes introduced in this PR.
• I've made sure all auto checks have passed.

[ekzhu]

Let's rename it return_error_types? This makes it align with the return_type property that already exists.

Let's also add API doc comment to each field of Tool and BaseTool as a good Samaritan 😄

[wistuba]

Let's also add API doc comment to each field of Tool and BaseTool as a good Samaritan

Not sure what you mean by this. Do you want me to add docstrings to every function in Tool/BaseTool?

[ekzhu]

Yes. I was asking if you would be willing to add the docstrings.

[wistuba]

I think there was some confusion on the Issue regarding what this feature should do. Do we want to keep adding the error types to the Tool or is there now a preference to add it to the agent? Former has the advantage that you can deal differently with each tool, latter allows you to only define it once and it is applied automatically for every tool.

[ekzhu]

So sorry I am going back-and-forth here.

My initial thought for #5274 is that we want to configure how FunctionTool behaves when it comes to what error to catch and return the error message as the result, and what errors to raise up the stack. So that the actual function passed in does not need to implement special handling for errors. For example, a function that raises an HTTP 401 error, the FunctionTool that wraps it can return back to the caller a string result rather than raising the error up the stack, without changing the implementation of the underlying function that performs the HTTP call.

However, I think there could be some issues:

1. The common usage case of BaseTool is not to execute the tool directly but through an agent. So we need to make BaseTool works well with FunctionExecutionResult, which has is_error field to indicate error. If the BaseTool returns a string, there is no way to know whether is_error field should be set. So we still need a way to indicate whether an error has occurred or not.
2. The added return_error_types field addresses the problem but doesn't solve it cleanly, because we now still need to catch the error and see if it is part of the return_error_types and then make a decision on whether to return the message or raise. It feels strange, because the class that is responsible for using the property is not the owner of the property.

So I am more inclined to configure how the errors raised from BaseTool is managed through the caller of BaseTool. So in this case it should be the agent's responsibility to manage it.

So, we need to update AssistantAgent class regarding to how tool errors are handled.

However, I do think we need to wait for a bit because it is an important API change, and overlaps with concepts discussed in #4721. Let's wait on this PR and consider multiple ideas together.

[wistuba]

My proposal is to pass the return_error_types to the AssistantAgent. Then the AssistantAgent calls the tools and catches all return_error_types. I don't see why this change will impact the PR you've mentioned. In any case, let me know how to progress on this.

[ekzhu]

In any case, let me know how to progress on this.

Let's pause this for now. There are a number of things related to tool calls such as #5510 #4721 and this one. I think we need to design this a bit. Would you like to join our discord for more interactive discussion?