Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 34 additions & 10 deletions packages/ai-proxy/src/mcp-client.ts
Original file line number Diff line number Diff line change
@@ -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<MultiServerMCPClient['config'], 'mcpServers'>;
Expand Down Expand Up @@ -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 });
}
}),
Expand All @@ -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}`,
);
Expand All @@ -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);
}
}
}
Expand All @@ -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.`,
);
Expand Down
65 changes: 65 additions & 0 deletions packages/ai-proxy/test/mcp-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
Loading