Migrate to coreEvents/debugLogger (#12107)

This commit is contained in:
Tommaso Sciortino
2025-10-27 16:46:35 -07:00
committed by GitHub
parent e9f8ccd51a
commit 85f3a8c210
6 changed files with 173 additions and 218 deletions
@@ -13,6 +13,7 @@ import {
} from './mcp-client.js';
import { getErrorMessage } from '../utils/errors.js';
import type { EventEmitter } from 'node:events';
import { coreEvents } from '../utils/events.js';
/**
* Manages the lifecycle of multiple MCP clients, including local child processes.
@@ -70,10 +71,12 @@ export class McpClientManager {
} catch (error) {
this.eventEmitter?.emit('mcp-client-update', this.clients);
// Log the error but don't let a single failed server stop the others
console.error(
coreEvents.emitFeedback(
'error',
`Error during discovery for server '${name}': ${getErrorMessage(
error,
)}`,
error,
);
}
});
+10 -5
View File
@@ -29,6 +29,7 @@ import type { ToolRegistry } from './tool-registry.js';
import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { coreEvents } from '../utils/events.js';
vi.mock('@modelcontextprotocol/sdk/client/stdio.js');
vi.mock('@modelcontextprotocol/sdk/client/index.js');
@@ -37,6 +38,12 @@ vi.mock('../mcp/oauth-provider.js');
vi.mock('../mcp/oauth-token-storage.js');
vi.mock('../mcp/oauth-utils.js');
vi.mock('../utils/events.js', () => ({
coreEvents: {
emitFeedback: vi.fn(),
},
}));
describe('mcp-client', () => {
let workspaceContext: WorkspaceContext;
let testWorkspace: string;
@@ -164,9 +171,6 @@ describe('mcp-client', () => {
});
it('should handle errors when discovering prompts', async () => {
const consoleErrorSpy = vi
.spyOn(console, 'error')
.mockImplementation(() => {});
const mockedClient = {
connect: vi.fn(),
discover: vi.fn(),
@@ -200,10 +204,11 @@ describe('mcp-client', () => {
await expect(client.discover({} as Config)).rejects.toThrow(
'No prompts or tools found on the server.',
);
expect(consoleErrorSpy).toHaveBeenCalledWith(
expect(coreEvents.emitFeedback).toHaveBeenCalledWith(
'error',
`Error discovering prompts from test-server: Test error`,
expect.any(Error),
);
consoleErrorSpy.mockRestore();
});
it('should not discover tools if server does not support them', async () => {
+102 -135
View File
@@ -42,6 +42,7 @@ import type {
} from '../utils/workspaceContext.js';
import type { ToolRegistry } from './tool-registry.js';
import { debugLogger } from '../utils/debugLogger.js';
import { coreEvents } from '../utils/events.js';
export const MCP_DEFAULT_TIMEOUT_MSEC = 10 * 60 * 1000; // default to 10 minutes
@@ -119,7 +120,11 @@ export class McpClient {
return;
}
if (originalOnError) originalOnError(error);
console.error(`MCP ERROR (${this.serverName}):`, error.toString());
coreEvents.emitFeedback(
'error',
`MCP ERROR (${this.serverName})`,
error,
);
this.updateStatus(MCPServerStatus.DISCONNECTED);
};
this.updateStatus(MCPServerStatus.CONNECTED);
@@ -340,8 +345,9 @@ async function handleAutomaticOAuth(
}
if (!oauthConfig) {
console.error(
`❌ Could not configure OAuth for '${mcpServerName}' - please authenticate manually with /mcp auth ${mcpServerName}`,
coreEvents.emitFeedback(
'error',
`Could not configure OAuth for '${mcpServerName}' - please authenticate manually with /mcp auth ${mcpServerName}`,
);
return false;
}
@@ -370,8 +376,10 @@ async function handleAutomaticOAuth(
);
return true;
} catch (error) {
console.error(
coreEvents.emitFeedback(
'error',
`Failed to handle automatic OAuth for server '${mcpServerName}': ${getErrorMessage(error)}`,
error,
);
return false;
}
@@ -420,8 +428,10 @@ async function createTransportWithOAuth(
return null;
} catch (error) {
console.error(
coreEvents.emitFeedback(
'error',
`Failed to create OAuth transport for server '${mcpServerName}': ${getErrorMessage(error)}`,
error,
);
return null;
}
@@ -520,7 +530,7 @@ export async function connectAndDiscover(
);
mcpClient.onerror = (error) => {
console.error(`MCP ERROR (${mcpServerName}):`, error.toString());
coreEvents.emitFeedback('error', `MCP ERROR (${mcpServerName}):`, error);
updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED);
};
@@ -553,10 +563,12 @@ export async function connectAndDiscover(
if (mcpClient) {
mcpClient.close();
}
console.error(
coreEvents.emitFeedback(
'error',
`Error connecting to MCP server '${mcpServerName}': ${getErrorMessage(
error,
)}`,
error,
);
updateMCPServerStatus(mcpServerName, MCPServerStatus.DISCONNECTED);
}
@@ -614,10 +626,12 @@ export async function discoverTools(
),
);
} catch (error) {
console.error(
coreEvents.emitFeedback(
'error',
`Error discovering tool: '${
funcDecl.name
}' from MCP server '${mcpServerName}': ${(error as Error).message}`,
error,
);
}
}
@@ -627,10 +641,12 @@ export async function discoverTools(
error instanceof Error &&
!error.message?.includes('Method not found')
) {
console.error(
coreEvents.emitFeedback(
'error',
`Error discovering tools from ${mcpServerName}: ${getErrorMessage(
error,
)}`,
error,
);
}
return [];
@@ -674,10 +690,12 @@ export async function discoverPrompts(
error instanceof Error &&
!error.message?.includes('Method not found')
) {
console.error(
coreEvents.emitFeedback(
'error',
`Error discovering prompts from ${mcpServerName}: ${getErrorMessage(
error,
)}`,
error,
);
}
return [];
@@ -717,10 +735,12 @@ export async function invokeMcpPrompt(
error instanceof Error &&
!error.message?.includes('Method not found')
) {
console.error(
coreEvents.emitFeedback(
'error',
`Error invoking prompt '${promptName}' from ${mcpServerName} ${promptParams}: ${getErrorMessage(
error,
)}`,
error,
);
}
throw error;
@@ -842,12 +862,14 @@ export async function connectToMcpServer(
},
);
if (hasStoredTokens) {
debugLogger.log(
coreEvents.emitFeedback(
'error',
`Stored OAuth token for SSE server '${mcpServerName}' was rejected. ` +
`Please re-authenticate using: /mcp auth ${mcpServerName}`,
);
} else {
debugLogger.log(
coreEvents.emitFeedback(
'error',
`401 error received for SSE server '${mcpServerName}' without OAuth configuration. ` +
`Please authenticate using: /mcp auth ${mcpServerName}`,
);
@@ -935,49 +957,27 @@ export async function connectToMcpServer(
accessToken,
);
if (oauthTransport) {
try {
await mcpClient.connect(oauthTransport, {
timeout:
mcpServerConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC,
});
// Connection successful with OAuth
return mcpClient;
} catch (retryError) {
console.error(
`Failed to connect with OAuth token: ${getErrorMessage(
retryError,
)}`,
);
throw retryError;
}
await mcpClient.connect(oauthTransport, {
timeout: mcpServerConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC,
});
// Connection successful with OAuth
return mcpClient;
} else {
console.error(
`Failed to create OAuth transport for server '${mcpServerName}'`,
);
throw new Error(
`Failed to create OAuth transport for server '${mcpServerName}'`,
);
}
} else {
console.error(
`Failed to get OAuth token for server '${mcpServerName}'`,
);
throw new Error(
`Failed to get OAuth token for server '${mcpServerName}'`,
);
}
} else {
console.error(
`Failed to get credentials for server '${mcpServerName}' after successful OAuth authentication`,
);
throw new Error(
`Failed to get credentials for server '${mcpServerName}' after successful OAuth authentication`,
);
}
} else {
console.error(
`Failed to handle automatic OAuth for server '${mcpServerName}'`,
);
throw new Error(
`Failed to handle automatic OAuth for server '${mcpServerName}'`,
);
@@ -1002,12 +1002,14 @@ export async function connectToMcpServer(
},
);
if (hasStoredTokens) {
debugLogger.log(
coreEvents.emitFeedback(
'error',
`Stored OAuth token for SSE server '${mcpServerName}' was rejected. ` +
`Please re-authenticate using: /mcp auth ${mcpServerName}`,
);
} else {
debugLogger.log(
coreEvents.emitFeedback(
'error',
`401 error received for SSE server '${mcpServerName}' without OAuth configuration. ` +
`Please authenticate using: /mcp auth ${mcpServerName}`,
);
@@ -1030,116 +1032,85 @@ export async function connectToMcpServer(
);
const baseUrl = `${serverUrl.protocol}//${serverUrl.host}`;
try {
// Try to discover OAuth configuration from the base URL
const oauthConfig = await OAuthUtils.discoverOAuthConfig(baseUrl);
if (oauthConfig) {
debugLogger.log(
`Discovered OAuth configuration from base URL for server '${mcpServerName}'`,
);
// Try to discover OAuth configuration from the base URL
const oauthConfig = await OAuthUtils.discoverOAuthConfig(baseUrl);
if (oauthConfig) {
debugLogger.log(
`Discovered OAuth configuration from base URL for server '${mcpServerName}'`,
);
// Create OAuth configuration for authentication
const oauthAuthConfig = {
enabled: true,
authorizationUrl: oauthConfig.authorizationUrl,
tokenUrl: oauthConfig.tokenUrl,
scopes: oauthConfig.scopes || [],
};
// Create OAuth configuration for authentication
const oauthAuthConfig = {
enabled: true,
authorizationUrl: oauthConfig.authorizationUrl,
tokenUrl: oauthConfig.tokenUrl,
scopes: oauthConfig.scopes || [],
};
// Perform OAuth authentication
// Pass the server URL for proper discovery
const authServerUrl =
mcpServerConfig.httpUrl || mcpServerConfig.url;
debugLogger.log(
`Starting OAuth authentication for server '${mcpServerName}'...`,
);
const authProvider = new MCPOAuthProvider(
new MCPOAuthTokenStorage(),
);
await authProvider.authenticate(
// Perform OAuth authentication
// Pass the server URL for proper discovery
const authServerUrl =
mcpServerConfig.httpUrl || mcpServerConfig.url;
debugLogger.log(
`Starting OAuth authentication for server '${mcpServerName}'...`,
);
const authProvider = new MCPOAuthProvider(
new MCPOAuthTokenStorage(),
);
await authProvider.authenticate(
mcpServerName,
oauthAuthConfig,
authServerUrl,
);
// Retry connection with OAuth token
const tokenStorage = new MCPOAuthTokenStorage();
const credentials =
await tokenStorage.getCredentials(mcpServerName);
if (credentials) {
const authProvider = new MCPOAuthProvider(tokenStorage);
const accessToken = await authProvider.getValidToken(
mcpServerName,
oauthAuthConfig,
authServerUrl,
{
// Pass client ID if available
clientId: credentials.clientId,
},
);
// Retry connection with OAuth token
const tokenStorage = new MCPOAuthTokenStorage();
const credentials =
await tokenStorage.getCredentials(mcpServerName);
if (credentials) {
const authProvider = new MCPOAuthProvider(tokenStorage);
const accessToken = await authProvider.getValidToken(
if (accessToken) {
// Create transport with OAuth token
const oauthTransport = await createTransportWithOAuth(
mcpServerName,
{
// Pass client ID if available
clientId: credentials.clientId,
},
mcpServerConfig,
accessToken,
);
if (accessToken) {
// Create transport with OAuth token
const oauthTransport = await createTransportWithOAuth(
mcpServerName,
mcpServerConfig,
accessToken,
);
if (oauthTransport) {
try {
await mcpClient.connect(oauthTransport, {
timeout:
mcpServerConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC,
});
// Connection successful with OAuth
return mcpClient;
} catch (retryError) {
console.error(
`Failed to connect with OAuth token: ${getErrorMessage(
retryError,
)}`,
);
throw retryError;
}
} else {
console.error(
`Failed to create OAuth transport for server '${mcpServerName}'`,
);
throw new Error(
`Failed to create OAuth transport for server '${mcpServerName}'`,
);
}
if (oauthTransport) {
await mcpClient.connect(oauthTransport, {
timeout:
mcpServerConfig.timeout ?? MCP_DEFAULT_TIMEOUT_MSEC,
});
// Connection successful with OAuth
return mcpClient;
} else {
console.error(
`Failed to get OAuth token for server '${mcpServerName}'`,
);
throw new Error(
`Failed to get OAuth token for server '${mcpServerName}'`,
`Failed to create OAuth transport for server '${mcpServerName}'`,
);
}
} else {
console.error(
`Failed to get stored credentials for server '${mcpServerName}'`,
);
throw new Error(
`Failed to get stored credentials for server '${mcpServerName}'`,
`Failed to get OAuth token for server '${mcpServerName}'`,
);
}
} else {
console.error(
`❌ Could not configure OAuth for '${mcpServerName}' - please authenticate manually with /mcp auth ${mcpServerName}`,
);
throw new Error(
`OAuth configuration failed for '${mcpServerName}'. Please authenticate manually with /mcp auth ${mcpServerName}`,
`Failed to get stored credentials for server '${mcpServerName}'`,
);
}
} catch (discoveryError) {
console.error(
`OAuth discovery failed for '${mcpServerName}' - please authenticate manually with /mcp auth ${mcpServerName}`,
} else {
throw new Error(
`OAuth configuration failed for '${mcpServerName}'. Please authenticate manually with /mcp auth ${mcpServerName}`,
);
throw discoveryError;
}
} else {
console.error(
`❌ '${mcpServerName}' requires authentication but no OAuth configuration found`,
);
throw new Error(
`MCP server '${mcpServerName}' requires authentication. Please configure OAuth or check server settings.`,
);
@@ -1239,10 +1210,6 @@ export async function createTransport(
);
if (!accessToken) {
console.error(
`MCP server '${mcpServerName}' requires OAuth authentication. ` +
`Please authenticate using the /mcp auth command.`,
);
throw new Error(
`MCP server '${mcpServerName}' requires OAuth authentication. ` +
`Please authenticate using the /mcp auth command.`,
+6 -2
View File
@@ -23,6 +23,7 @@ import { safeJsonStringify } from '../utils/safeJsonStringify.js';
import type { EventEmitter } from 'node:events';
import type { MessageBus } from '../confirmation-bus/message-bus.js';
import { debugLogger } from '../utils/debugLogger.js';
import { coreEvents } from '../utils/events.js';
type ToolParams = Record<string, unknown>;
@@ -350,8 +351,11 @@ export class ToolRegistry {
}
if (code !== 0) {
console.error(`Command failed with code ${code}`);
console.error(stderr);
coreEvents.emitFeedback(
'error',
`Tool discovery command failed with code ${code}.`,
stderr,
);
return reject(
new Error(`Tool discovery command failed with exit code ${code}`),
);