feat: add NewToolResultError #87
Conversation
WalkthroughThe changes introduce a new function, Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
daimatz
force-pushed
the
add-ResultError
branch
from
205a459 to
591a196
Compare
|
I feel like this is the return of an error pattern that was previously documented/recommended. Extract from the README in 0.10.0: func helloHandler(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
name, ok := request.Params.Arguments["name"].(string)
if !ok {
return mcp.NewToolResultError("name must be a string"), nil
}
return mcp.NewToolResultText(fmt.Sprintf("Hello, %s!", name)), nil
}However, the current version of the README favors the usage of s.AddTool(calculatorTool, func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) {
op := request.Params.Arguments["operation"].(string)
x := request.Params.Arguments["x"].(float64)
y := request.Params.Arguments["y"].(float64)
var result float64
switch op {
case "add":
result = x + y
case "subtract":
result = x - y
case "multiply":
result = x * y
case "divide":
if y == 0 {
return nil, errors.New("Division by zero is not allowed")
}
result = x / y
}
return mcp.FormatNumberResult(result), nil
}) |
|
@deviantony I agree with your note, I was also wondering if the My use case for MCP is to integrate an API which is known to sometimes fail or timeout. I'd rather have the LLM report back to the user in a human-readable way, what the problem is. |
|
I don't know the history, but according to the MCP protocol, it states that tool-generated errors and protocol-level errors should be separated. In my personal use case, I use LibreChat to have an LLM generate SQL, which is then run on BigQuery via MCP. The problem is, the SQL that the LLM writes often contains syntax errors, resulting in SQL execution errors. In such cases, rather than returning a protocol-level error, we need to inform the LLM that its input is incorrect and that it should fix the syntax. I believe this is exactly the kind of situation where the isError field should be used. In fact, instead of doing something like |
|
Good to hear @daimatz, I believe that it might be valuable to update the documentation in the README to match this instead of using the I wonder when are you supposed to raise an error as a protocol level error though, any example that could be added as well? I could open a PR afterwards. |
|
Thanks, adding such an example to the README sounds like a great idea. MCP states:
This implies that such situations should be treated as protocol-level errors. |
|
I've opened #140 to follow up on this |
Added the NewToolResultError method.
In MCP, when an error occurs during a tool invocation, it is defined that the error message should be included in the contents of the Result object, rather than as a protocol level error.
Summary by CodeRabbit