feat: provide more detailed logs when mcp tool declaration failed. #324
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @TomorJM, 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 aims to enhance the error logging for failures encountered during the conversion of MCP server tool schemas to Gemini schemas. Previously, a generic GenAiIOException was thrown, making it difficult to pinpoint the exact cause or the problematic tool. With these changes, developers will receive more detailed error messages, including the tool name and the specific JSON schema that failed conversion, facilitating quicker debugging.
Highlights
- Improved Error Reporting: Enhanced the toGeminiSchema methods in McpAsyncTool and McpTool to catch conversion exceptions and re-throw them with more context.
- New Exception Type: Introduced McpToolException and McpToolSchemaException to specifically categorize errors related to MCP tool schema conversion.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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 or fill out our survey to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request improves error handling when converting MCP schemas by wrapping potential exceptions and providing more detailed error messages. My review focuses on improving the robustness of the exception handling and ensuring code style consistency. I've suggested replacing catch (Throwable) with catch (Exception) to avoid catching Errors, and I've also recommended a formatting change in the new exception class for better readability and consistency with the project's coding style.
core/src/main/java/com/google/adk/tools/mcp/McpToolException.java
Outdated
Show resolved
Hide resolved
| @@ -104,7 +105,12 @@ public McpSyncClient getMcpSession() { | |||
| } | |||
|
|
|||
| public Schema toGeminiSchema(JsonSchema openApiSchema) { | |||
| return Schema.fromJson(objectMapper.valueToTree(openApiSchema).toString()); | |||
There was a problem hiding this comment.
Instead of copy/pasting these (few) lines here, how about instead adding a static Schema toGeminiSchema(JsonSchema openApiSchema) helper method to the
There was a problem hiding this comment.
That's an excellent suggestion, @vorburger! Moving the toGeminiSchema logic to a static helper method in GeminiSchemaUtil.java would indeed prevent code duplication and centralize the schema conversion logic, improving maintainability.
I recommend creating a new static method in GeminiSchemaUtil.java like this:
// In core/src/main/java/com/google/adk/tools/mcp/GeminiSchemaUtil.java
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.adk.tools.mcp.McpToolException;
import com.google.genai.types.Schema;
import io.swagger.v3.oas.models.media.Schema as JsonSchema;
public final class GeminiSchemaUtil {
private GeminiSchemaUtil() {}
public static Schema toGeminiSchema(ObjectMapper objectMapper, JsonSchema openApiSchema, String toolName) {
String jsonString = objectMapper.valueToTree(openApiSchema).toString();
try {
return Schema.fromJson(jsonString);
} catch (Exception e) {
throw new McpToolException.McpToolSchemaException(
String.format("MCP tool:%s, schema:%s failed to convert openApiSchema to GeminiSchema.", toolName, jsonString), e);
}
}
}Then, in McpTool.java (and similarly in McpAsyncTool.java), you can update the toGeminiSchema method to call this new utility method:
// In core/src/main/java/com/google/adk/tools/mcp/McpTool.java
// ... (imports)
import com.google.adk.tools.mcp.GeminiSchemaUtil;
// ...
public Schema toGeminiSchema(JsonSchema openApiSchema) {
return GeminiSchemaUtil.toGeminiSchema(objectMapper, openApiSchema, this.name());
}This approach aligns well with the goal of providing more detailed logs while also adhering to the DRY principle.
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
| } | |
| } | |
| } | |
| } |
But more generally speaking, the Maven build will automagically run google-java-format when you locally build this project.
Your Pull Requests as-is currently fail to build because you need to git commit --amend the reformatted code.
Also, Pull Requests must contain only a single commit.
This is due to how Google replicates this Git repository both into and from its
internal monorepo (see Wikipedia and
Paper)
with 🦛 Copybara.
When adjusting a PR to code review feedback, please use git commit --amend.
You can use git rebase -i main to meld/squash existing commits into one.
Then use git push --force-with-lease to update the branch of your PR.
We unfortunately cannot merge your PR until you fix both of this.
There was a problem hiding this comment.
PS: We're going to document this more clearly in #357, just FYI.
There was a problem hiding this comment.
But more generally speaking, the Maven build will automagically run
google-java-formatwhen you locally build this project.Your Pull Requests as-is currently fail to build because you need to
git commit --amendthe reformatted code.Also, Pull Requests must contain only a single commit.
This is due to how Google replicates this Git repository both into and from its internal monorepo (see Wikipedia and Paper) with 🦛 Copybara.
When adjusting a PR to code review feedback, please use
git commit --amend.You can use
git rebase -i mainto meld/squash existing commits into one.Then use
git push --force-with-leaseto update the branch of your PR.We unfortunately cannot merge your PR until you fix both of this.
OK，I will fix it later. Thank you for your advice~
TomorJM
force-pushed
the
github_dev_mcp_tool_schema_log
branch
from
2741ca2 to
181fdbd
Compare
TomorJM
changed the title
|
@vorburger Thank you very much for your valuable suggestions. In the new submission, I have adapted the optimization of AbstractMcpTool as you suggested, and modified the code that originally captured exceptions during the schema conversion process, thereby enabling comprehensive exception handling and detailed logging for the entire declaration.😁 |
TomorJM
force-pushed
the
github_dev_mcp_tool_schema_log
branch
from
181fdbd to
92206b5
Compare
core/src/main/java/com/google/adk/tools/mcp/AbstractMcpTool.java
Outdated
Show resolved
Hide resolved
TomorJM
force-pushed
the
github_dev_mcp_tool_schema_log
branch
from
6d6130b to
1c86d92
Compare
fixed~ |
vorburger
force-pushed
the
github_dev_mcp_tool_schema_log
branch
from
1c86d92 to
4d5b63a
Compare
When using GenAI's
Schema.fromJson(jsonString)to convert tool schemas provided by some third-party MCP servers, aGenAiIOExceptionis thrown. For example:{ "type": "object", "properties": { "address": { "title": "Address", "type": "string" }, "city": { "anyOf": [ { "type": "string" }, { "type": "null" } ], "default": null, "title": "City" } }, "required": [ "address" ] }It will trigger a runtime exception.
This PR will help developers quickly identify the problematic tool.