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', () => {