fix: Do not throw on tool errors, instead return error message (#17558)

packages/@n8n/nodes-langchain/nodes/mcp/McpClientTool/McpClientTool.node.test.ts

@@ -2,6 +2,7 @@ import { Client } from '@modelcontextprotocol/sdk/client/index.js';
 import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js';
 import { mock } from 'jest-mock-extended';
 import {
+	NodeConnectionTypes,
 	NodeOperationError,
 	type ILoadOptionsFunctions,
 	type INode,
@@ -284,5 +285,78 @@ describe('McpClientTool', () => {
 				headers: { Accept: 'text/event-stream', Authorization: 'Bearer my-token' },
 			});
 		});
+
+		it('should successfully execute a tool', async () => {
+			jest.spyOn(Client.prototype, 'connect').mockResolvedValue();
+			jest.spyOn(Client.prototype, 'callTool').mockResolvedValue({ content: 'Sunny' });
+			jest.spyOn(Client.prototype, 'listTools').mockResolvedValue({
+				tools: [
+					{
+						name: 'Weather Tool',
+						description: 'Gets the current weather',
+						inputSchema: { type: 'object', properties: { location: { type: 'string' } } },
+					},
+				],
+			});
+
+			const supplyDataResult = await new McpClientTool().supplyData.call(
+				mock<ISupplyDataFunctions>({
+					getNode: jest.fn(() =>
+						mock<INode>({
+							typeVersion: 1,
+						}),
+					),
+					logger: { debug: jest.fn(), error: jest.fn() },
+					addInputData: jest.fn(() => ({ index: 0 })),
+				}),
+				0,
+			);
+
+			expect(supplyDataResult.closeFunction).toBeInstanceOf(Function);
+			expect(supplyDataResult.response).toBeInstanceOf(McpToolkit);
+
+			const tools = (supplyDataResult.response as McpToolkit).getTools();
+			const toolResult = await tools[0].invoke({ location: 'Berlin' });
+			expect(toolResult).toEqual('Sunny');
+		});
+
+		it('should handle tool errors', async () => {
+			jest.spyOn(Client.prototype, 'connect').mockResolvedValue();
+			jest
+				.spyOn(Client.prototype, 'callTool')
+				.mockResolvedValue({ isError: true, content: [{ text: 'Weather unknown at location' }] });
+			jest.spyOn(Client.prototype, 'listTools').mockResolvedValue({
+				tools: [
+					{
+						name: 'Weather Tool',
+						description: 'Gets the current weather',
+						inputSchema: { type: 'object', properties: { location: { type: 'string' } } },
+					},
+				],
+			});
+
+			const supplyDataFunctions = mock<ISupplyDataFunctions>({
+				getNode: jest.fn(() =>
+					mock<INode>({
+						typeVersion: 1,
+					}),
+				),
+				logger: { debug: jest.fn(), error: jest.fn() },
+				addInputData: jest.fn(() => ({ index: 0 })),
+			});
+			const supplyDataResult = await new McpClientTool().supplyData.call(supplyDataFunctions, 0);
+
+			expect(supplyDataResult.closeFunction).toBeInstanceOf(Function);
+			expect(supplyDataResult.response).toBeInstanceOf(McpToolkit);
+
+			const tools = (supplyDataResult.response as McpToolkit).getTools();
+			const toolResult = await tools[0].invoke({ location: 'Berlin' });
+			expect(toolResult).toEqual('Weather unknown at location');
+			expect(supplyDataFunctions.addOutputData).toHaveBeenCalledWith(
+				NodeConnectionTypes.AiTool,
+				0,
+				new NodeOperationError(supplyDataFunctions.getNode(), 'Weather unknown at location'),
+			);
+		});
 	});
 });

packages/@n8n/nodes-langchain/nodes/mcp/McpClientTool/McpClientTool.node.ts

@@ -1,3 +1,5 @@
+import { logWrapper } from '@utils/logWrapper';
+import { getConnectionHintNoticeField } from '@utils/sharedFields';
 import {
 	NodeConnectionTypes,
 	NodeOperationError,
@@ -7,9 +9,6 @@ import {
 	type SupplyData,
 } from 'n8n-workflow';
 
-import { logWrapper } from '@utils/logWrapper';
-import { getConnectionHintNoticeField } from '@utils/sharedFields';
-
 import { getTools } from './loadOptions';
 import type { McpServerTransport, McpAuthenticationOption, McpToolIncludeMode } from './types';
 import {
@@ -294,11 +293,10 @@ export class McpClientTool implements INodeType {
 			logWrapper(
 				mcpToolToDynamicTool(
 					tool,
-					createCallTool(tool.name, client.result, (error) => {
+					createCallTool(tool.name, client.result, (errorMessage) => {
+						const error = new NodeOperationError(node, errorMessage, { itemIndex });
+						void this.addOutputData(NodeConnectionTypes.AiTool, itemIndex, error);
 						this.logger.error(`McpClientTool: Tool "${tool.name}" failed to execute`, { error });
-						throw new NodeOperationError(node, `Failed to execute tool "${tool.name}"`, {
-							description: error,
-						});
 					}),
 				),
 				this,

packages/@n8n/nodes-langchain/nodes/mcp/McpClientTool/utils.ts

@@ -3,6 +3,7 @@ import { Client } from '@modelcontextprotocol/sdk/client/index.js';
 import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js';
 import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js';
 import { CompatibilityCallToolResultSchema } from '@modelcontextprotocol/sdk/types.js';
+import { convertJsonSchemaToZod } from '@utils/schemaParsing';
 import { Toolkit } from 'langchain/agents';
 import {
 	createResultError,
@@ -13,12 +14,10 @@ import {
 } from 'n8n-workflow';
 import { z } from 'zod';
 
-import { convertJsonSchemaToZod } from '@utils/schemaParsing';
-
 import type {
 	McpAuthenticationOption,
-	McpTool,
 	McpServerTransport,
+	McpTool,
 	McpToolIncludeMode,
 } from './types';
 
@@ -78,17 +77,24 @@ export const getErrorDescriptionFromToolCall = (result: unknown): string | undef
 };
 
 export const createCallTool =
-	(name: string, client: Client, onError: (error: string | undefined) => void) =>
-	async (args: IDataObject) => {
+	(name: string, client: Client, onError: (error: string) => void) => async (args: IDataObject) => {
 		let result: Awaited<ReturnType<Client['callTool']>>;
+
+		function handleError(error: unknown) {
+			const errorDescription =
+				getErrorDescriptionFromToolCall(error) ?? `Failed to execute tool "${name}"`;
+			onError(errorDescription);
+			return errorDescription;
+		}
+
 		try {
 			result = await client.callTool({ name, arguments: args }, CompatibilityCallToolResultSchema);
 		} catch (error) {
-			return onError(getErrorDescriptionFromToolCall(error));
+			return handleError(error);
 		}
 
 		if (result.isError) {
-			return onError(getErrorDescriptionFromToolCall(result));
+			return handleError(result);
 		}
 
 		if (result.toolResult !== undefined) {
@@ -105,7 +111,7 @@ export const createCallTool =
 export function mcpToolToDynamicTool(
 	tool: McpTool,
 	onCallTool: DynamicStructuredToolInput['func'],
-): DynamicStructuredTool<z.ZodObject<any, any, any, any>> {
+): DynamicStructuredTool {
 	const rawSchema = convertJsonSchemaToZod(tool.inputSchema);
 
 	// Ensure we always have an object schema for structured tools
@@ -122,7 +128,7 @@ export function mcpToolToDynamicTool(
 }
 
 export class McpToolkit extends Toolkit {
-	constructor(public tools: Array<DynamicStructuredTool<z.ZodObject<any, any, any, any>>>) {
+	constructor(public tools: DynamicStructuredTool[]) {
 		super();
 	}
 }