From d06e17fbd9d08ac5efccf0c2b4d5fd4f703b15c7 Mon Sep 17 00:00:00 2001
From: Gal Zahavi <38544478+galz10@users.noreply.github.com>
Date: Wed, 30 Jul 2025 17:16:21 -0700
Subject: [PATCH] Improve error message for discoverTools function (#4157)

---
 packages/core/src/tools/mcp-client.test.ts | 49 ++++++++++++++++++++++
 packages/core/src/tools/mcp-client.ts      | 36 +++++++++-------
 2 files changed, 71 insertions(+), 14 deletions(-)

diff --git a/packages/core/src/tools/mcp-client.test.ts b/packages/core/src/tools/mcp-client.test.ts
index 4560982c..a8289d3b 100644
--- a/packages/core/src/tools/mcp-client.test.ts
+++ b/packages/core/src/tools/mcp-client.test.ts
@@ -21,11 +21,14 @@ import { GoogleCredentialProvider } from '../mcp/google-auth-provider.js';
 import { AuthProviderType } from '../config/config.js';
 import { PromptRegistry } from '../prompts/prompt-registry.js';
 
+import { DiscoveredMCPTool } from './mcp-tool.js';
+
 vi.mock('@modelcontextprotocol/sdk/client/stdio.js');
 vi.mock('@modelcontextprotocol/sdk/client/index.js');
 vi.mock('@google/genai');
 vi.mock('../mcp/oauth-provider.js');
 vi.mock('../mcp/oauth-token-storage.js');
+vi.mock('./mcp-tool.js');
 
 describe('mcp-client', () => {
   afterEach(() => {
@@ -50,6 +53,52 @@ describe('mcp-client', () => {
       expect(tools.length).toBe(1);
       expect(mockedMcpToTool).toHaveBeenCalledOnce();
     });
+
+    it('should log an error if there is an error discovering a tool', async () => {
+      const mockedClient = {} as unknown as ClientLib.Client;
+      const consoleErrorSpy = vi
+        .spyOn(console, 'error')
+        .mockImplementation(() => {
+          // no-op
+        });
+
+      const testError = new Error('Invalid tool name');
+      vi.mocked(DiscoveredMCPTool).mockImplementation(
+        (
+          _mcpCallableTool: GenAiLib.CallableTool,
+          _serverName: string,
+          name: string,
+        ) => {
+          if (name === 'invalid tool name') {
+            throw testError;
+          }
+          return { name: 'validTool' } as DiscoveredMCPTool;
+        },
+      );
+
+      vi.mocked(GenAiLib.mcpToTool).mockReturnValue({
+        tool: () =>
+          Promise.resolve({
+            functionDeclarations: [
+              {
+                name: 'validTool',
+              },
+              {
+                name: 'invalid tool name', // this will fail validation
+              },
+            ],
+          }),
+      } as unknown as GenAiLib.CallableTool);
+
+      const tools = await discoverTools('test-server', {}, mockedClient);
+
+      expect(tools.length).toBe(1);
+      expect(tools[0].name).toBe('validTool');
+      expect(consoleErrorSpy).toHaveBeenCalledOnce();
+      expect(consoleErrorSpy).toHaveBeenCalledWith(
+        `Error discovering tool: 'invalid tool name' from MCP server 'test-server': ${testError.message}`,
+      );
+    });
   });
 
   describe('discoverPrompts', () => {
diff --git a/packages/core/src/tools/mcp-client.ts b/packages/core/src/tools/mcp-client.ts
index b87b2124..f9ccc380 100644
--- a/packages/core/src/tools/mcp-client.ts
+++ b/packages/core/src/tools/mcp-client.ts
@@ -428,21 +428,29 @@ export async function discoverTools(
 
     const discoveredTools: DiscoveredMCPTool[] = [];
     for (const funcDecl of tool.functionDeclarations) {
-      if (!isEnabled(funcDecl, mcpServerName, mcpServerConfig)) {
-        continue;
-      }
+      try {
+        if (!isEnabled(funcDecl, mcpServerName, mcpServerConfig)) {
+          continue;
+        }
 
-      discoveredTools.push(
-        new DiscoveredMCPTool(
-          mcpCallableTool,
-          mcpServerName,
-          funcDecl.name!,
-          funcDecl.description ?? '',
-          funcDecl.parametersJsonSchema ?? { type: 'object', properties: {} },
-          mcpServerConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC,
-          mcpServerConfig.trust,
-        ),
-      );
+        discoveredTools.push(
+          new DiscoveredMCPTool(
+            mcpCallableTool,
+            mcpServerName,
+            funcDecl.name!,
+            funcDecl.description ?? '',
+            funcDecl.parametersJsonSchema ?? { type: 'object', properties: {} },
+            mcpServerConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC,
+            mcpServerConfig.trust,
+          ),
+        );
+      } catch (error) {
+        console.error(
+          `Error discovering tool: '${
+            funcDecl.name
+          }' from MCP server '${mcpServerName}': ${(error as Error).message}`,
+        );
+      }
     }
     return discoveredTools;
   } catch (error) {