fix: handle errors when toolExecution fails for functionToolCallback

[emaarco]

FunctionToolCallback does not wrap exceptions in ToolExecutionException

fixes issue 2857

Description

The FunctionToolCallback does not properly handle exceptions thrown by user-defined tool functions. Unlike other tool callback implementations (such as SyncMcpToolCallback), it does not wrap exceptions in ToolExecutionException, which prevents proper error handling by the framework's exception processing mechanisms.

Current Behavior

When a tool function throws an exception, it propagates uncaught through the FunctionToolCallback.call() method:

@Override
public String call(String toolInput, @Nullable ToolContext toolContext) {
    Assert.hasText(toolInput, "toolInput cannot be null or empty");
    logger.debug("Starting execution of tool: {}", this.toolDefinition.name());
    I request = JsonParser.fromJson(toolInput, this.toolInputType);
    O response = this.toolFunction.apply(request, toolContext); // Exception not caught
    logger.debug("Successful execution of tool: {}", this.toolDefinition.name());
    return this.toolCallResultConverter.convert(response, null);
}

Expected Behavior

Exceptions should be caught and wrapped in ToolExecutionException to maintain consistency with other tool callback implementations and enable proper error processing.

Proposed Solution

Wrap the tool function execution in a try-catch block, similar to the implementation in SyncMcpToolCallback:

try {
    response = this.toolFunction.apply(request, toolContext);
} catch (Exception e) {
    logger.error("Error executing tool: {}", this.toolDefinition.name(), e);
    throw new ToolExecutionException(this.toolDefinition, e);
}

Alternatives / Contribution for other options

If this is not an appropriate way to solve this issue please let me know! :) I would be happy to also contribute more general solutions - like making the ToolExecutionExceptionProcessor more generic (so that it can handle all kind of exceptions - and not just ToolExecutionExceptions)

Impact

This change would:

• Enable error processing by the framework's exception processor (ToolExecutionExceptionProcessor)
• Improve debugging and error reporting for FunctionToolCallback execution failures

Example Use Case

@Bean
fun exampleTool(): FunctionToolCallback<Void, List<String>> {
    return FunctionToolCallback
        .builder("ExampleTool", exampleTool::performCall)
        .description("This is an example tool that returns a list of example strings.")
        .build()
}

class ExampleTool {
    fun performCall(): List<String> {
        val response = client.execute("query { exampleField }")
        if (response.hasErrors()) {
            throw RuntimeException("Error fetching example data: $response") // This should be wrapped
        } else {
            val data = response.getData("exampleField")
            return data as List<String>
        }
    }
}

Currently, the RuntimeException thrown in performCall() is not wrapped in a ToolExecutionException, making it difficult for error processors to handle tool-specific failures appropriately.

[YunKuiLu]

Related #3918

[sobychacko]

@emaarco Just merged the other PR - #3918. Is this PR still needed since both of them address the same issue? If so, please reopen and add more context. Thanks!

[emaarco]

@sobychacko - thank you. I'll have a look, but i think its solved.
Only aspect would be that it does not behave like "MethodToolCallback".
MethodToolCallback is only catching InvocationTargetException. The fix however is catching all Exception(s)

Moreover, i have the feeling that given the fact, that this "try-catch" logic is in multiple tool-implementations (like FunctionToolCallback, SyncMcpToolCallback & MethodToolCallback) one could think about modifying the ToolExecutionExceptionProcessor.

Whats your take on that? (can also open a dedicated issue on that - in case it does not already exist)