From 9d5c6389df453892bbcf5094fdce20d309ef9187 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Fri, 30 Jan 2026 16:49:09 +0100 Subject: [PATCH] feat(ai-proxy): log MCP unreachable server errors as Warn instead of Error When MCP servers are unreachable (connection refused, DNS not found, timeout, network unreachable), log as 'Warn' instead of 'Error' to reduce noise. All other errors (authentication, configuration, protocol errors) remain as 'Error'. Unreachable error codes: ECONNREFUSED, ENOTFOUND, ETIMEDOUT, ENETUNREACH, EHOSTUNREACH Co-Authored-By: Claude Opus 4.5 --- packages/ai-proxy/src/mcp-client.ts | 44 +++++++++++---- packages/ai-proxy/test/mcp-client.test.ts | 65 +++++++++++++++++++++++ 2 files changed, 99 insertions(+), 10 deletions(-) diff --git a/packages/ai-proxy/src/mcp-client.ts b/packages/ai-proxy/src/mcp-client.ts index b30212a8f..922af7b61 100644 --- a/packages/ai-proxy/src/mcp-client.ts +++ b/packages/ai-proxy/src/mcp-client.ts @@ -1,10 +1,28 @@ -import type { Logger } from '@forestadmin/datasource-toolkit'; +import type { Logger, LoggerLevel } from '@forestadmin/datasource-toolkit'; import { MultiServerMCPClient } from '@langchain/mcp-adapters'; import { McpConnectionError } from './types/errors'; import McpServerRemoteTool from './types/mcp-server-remote-tool'; +const UNREACHABLE_ERROR_CODES: string[] = [ + 'ECONNREFUSED', + 'ENOTFOUND', + 'ETIMEDOUT', + 'ENETUNREACH', + 'EHOSTUNREACH', +]; + +function isServerUnreachable(error: Error): boolean { + const { code } = error as NodeJS.ErrnoException; + + return !!code && UNREACHABLE_ERROR_CODES.includes(code); +} + +function getLogLevelForError(error: Error): LoggerLevel { + return isServerUnreachable(error) ? 'Warn' : 'Error'; +} + export type McpConfiguration = { configs: MultiServerMCPClient['config']['mcpServers']; } & Omit; @@ -39,7 +57,8 @@ export default class McpClient { ); this.tools.push(...extendedTools); } catch (error) { - this.logger?.('Error', `Error loading tools for ${name}`, error as Error); + const logLevel = getLogLevelForError(error as Error); + this.logger?.(logLevel, `Error loading tools for ${name}`, error as Error); errors.push({ server: name, error: error as Error }); } }), @@ -48,8 +67,10 @@ export default class McpClient { // Surface partial failures to provide better feedback if (errors.length > 0) { const errorMessage = errors.map(e => `${e.server}: ${e.error.message}`).join('; '); + const allConnectionErrors = errors.every(e => isServerUnreachable(e.error)); + const summaryLogLevel = allConnectionErrors ? 'Warn' : 'Error'; this.logger?.( - 'Error', + summaryLogLevel, `Failed to load tools from ${errors.length}/${Object.keys(this.mcpClients).length} ` + `MCP server(s): ${errorMessage}`, ); @@ -72,7 +93,8 @@ export default class McpClient { await this.closeConnections(); } catch (cleanupError) { // Log but don't throw - we don't want to mask the original connection error - this.logger?.('Error', 'Error during test connection cleanup', cleanupError as Error); + const logLevel = getLogLevelForError(cleanupError as Error); + this.logger?.(logLevel, 'Error during test connection cleanup', cleanupError as Error); } } } @@ -88,14 +110,16 @@ export default class McpClient { if (failures.length > 0) { failures.forEach(({ name, result }) => { - this.logger?.( - 'Error', - `Failed to close MCP connection for ${name}`, - (result as PromiseRejectedResult).reason, - ); + const error = (result as PromiseRejectedResult).reason; + const logLevel = getLogLevelForError(error); + this.logger?.(logLevel, `Failed to close MCP connection for ${name}`, error); }); + const allConnectionErrors = failures.every(({ result }) => + isServerUnreachable((result as PromiseRejectedResult).reason), + ); + const summaryLogLevel = allConnectionErrors ? 'Warn' : 'Error'; this.logger?.( - 'Error', + summaryLogLevel, `Failed to close ${failures.length}/${results.length} MCP connections. ` + `This may result in resource leaks.`, ); diff --git a/packages/ai-proxy/test/mcp-client.test.ts b/packages/ai-proxy/test/mcp-client.test.ts index caa09eaf2..bb12fbf6d 100644 --- a/packages/ai-proxy/test/mcp-client.test.ts +++ b/packages/ai-proxy/test/mcp-client.test.ts @@ -106,6 +106,71 @@ describe('McpClient', () => { expect(mcpClient.tools.length).toEqual(2); }); }); + + describe('error logging levels', () => { + function createSystemError(code: string, message: string): NodeJS.ErrnoException { + const error = new Error(message) as NodeJS.ErrnoException; + error.code = code; + + return error; + } + + it('should log unreachable server errors as Warn', async () => { + const loggerMock = jest.fn(); + const mcpClient = new McpClient(aConfig, loggerMock); + const unreachableError = createSystemError('ECONNREFUSED', 'Connection refused'); + getToolsMock.mockRejectedValue(unreachableError); + + await mcpClient.loadTools(); + + expect(loggerMock).toHaveBeenCalledWith( + 'Warn', + expect.stringContaining('Error loading tools for'), + unreachableError, + ); + expect(loggerMock).toHaveBeenCalledWith( + 'Warn', + expect.stringContaining('Failed to load tools from'), + ); + }); + + it('should log other errors as Error', async () => { + const loggerMock = jest.fn(); + const mcpClient = new McpClient(aConfig, loggerMock); + const otherError = new Error('Authentication failed'); + getToolsMock.mockRejectedValue(otherError); + + await mcpClient.loadTools(); + + expect(loggerMock).toHaveBeenCalledWith( + 'Error', + expect.stringContaining('Error loading tools for'), + otherError, + ); + expect(loggerMock).toHaveBeenCalledWith( + 'Error', + expect.stringContaining('Failed to load tools from'), + ); + }); + + it.each(['ECONNREFUSED', 'ENOTFOUND', 'ETIMEDOUT', 'ENETUNREACH', 'EHOSTUNREACH'])( + 'should log %s errors as Warn', + async errorCode => { + const loggerMock = jest.fn(); + const mcpClient = new McpClient(aConfig, loggerMock); + const error = createSystemError(errorCode, 'Connection error'); + getToolsMock.mockRejectedValue(error); + + await mcpClient.loadTools(); + + expect(loggerMock).toHaveBeenCalledWith( + 'Warn', + expect.stringContaining('Error loading tools for'), + error, + ); + }, + ); + }); }); describe('closeConnection', () => {