fix: serialize tool errors as strings in FunctionResponse #299
Conversation
Tool errors were previously serialized as error interfaces which marshal
to empty JSON objects ({}), making it impossible for LLMs to understand
what went wrong. This fix converts errors to strings using err.Error()
before including them in the FunctionResponse.
Changes:
- Modified callTool() in base_flow.go to call .Error() on error objects
- Added comprehensive tests for error serialization scenarios
- Covers tool execution errors, before/after callback errors
Fixes issue where LLMs received error: {} instead of meaningful error messages.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello @Omzee15, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves an issue where Large Language Models (LLMs) received uninformative empty JSON objects when tool calls resulted in errors. By explicitly converting error interfaces to their string representations, the change ensures that LLMs receive meaningful and actionable error messages, significantly improving their ability to understand and respond to tool failures. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively resolves the issue of tool errors being improperly serialized as empty JSON objects. The fix, which involves converting error objects to strings using .Error() before they are included in the FunctionResponse, is correct and well-implemented in base_flow.go. The addition of a comprehensive test suite in base_flow_test.go is a great enhancement, ensuring that various error scenarios are handled correctly and preventing future regressions. I have one minor suggestion to clean up some unused code in the new test file.
| testCases := []struct { | ||
| name string | ||
| toolError error | ||
| expectedError string | ||
| expectedPrefix string | ||
| }{ | ||
| { | ||
| name: "fmt.Errorf error", | ||
| toolError: fmt.Errorf("something went wrong with value %d", 42), | ||
| expectedError: "tool \"test-tool\" failed: something went wrong with value 42", | ||
| expectedPrefix: "tool \"test-tool\" failed:", | ||
| }, | ||
| { | ||
| name: "errors.New error", | ||
| toolError: errors.New("simple error message"), | ||
| expectedError: "tool \"test-tool\" failed: simple error message", | ||
| expectedPrefix: "tool \"test-tool\" failed:", | ||
| }, | ||
| { | ||
| name: "wrapped error", | ||
| toolError: fmt.Errorf("outer error: %w", errors.New("inner error")), | ||
| expectedError: "tool \"test-tool\" failed: outer error: inner error", | ||
| expectedPrefix: "tool \"test-tool\" failed:", | ||
| }, | ||
| } |
There was a problem hiding this comment.
The expectedPrefix field is defined in the testCases struct and initialized for each test case, but it is never actually used in the test logic. This unused code should be removed to improve clarity and maintainability.
testCases := []struct {
name string
toolError error
expectedError string
}{
{
name: "fmt.Errorf error",
toolError: fmt.Errorf("something went wrong with value %d", 42),
expectedError: "tool \"test-tool\" failed: something went wrong with value 42",
},
{
name: "errors.New error",
toolError: errors.New("simple error message"),
expectedError: "tool \"test-tool\" failed: simple error message",
},
{
name: "wrapped error",
toolError: fmt.Errorf("outer error: %w", errors.New("inner error")),
expectedError: "tool \"test-tool\" failed: outer error: inner error",
},
}1 participant
Tool errors were previously serialized as error interfaces which marshal to empty JSON objects ({}), making it impossible for LLMs to understand what went wrong. This fix converts errors to strings using err.Error() before including them in the FunctionResponse.
Changes:
Fixes issue where LLMs received error: {} instead of meaningful error messages.