From 59dd7d7eea3a7fa97f68cfd7069667aa01f855a4 Mon Sep 17 00:00:00 2001 From: Joaquin Coromina Date: Sat, 17 May 2025 18:21:55 -0400 Subject: [PATCH] Added proper error handling for config file parsing errors --- .../src/void-settings-tsx/MCPServersList.tsx | 63 +++++++++++-------- .../contrib/void/common/mcpService.ts | 36 ++++++++--- .../contrib/void/common/mcpServiceTypes.ts | 8 +++ 3 files changed, 74 insertions(+), 33 deletions(-) diff --git a/src/vs/workbench/contrib/void/browser/react/src/void-settings-tsx/MCPServersList.tsx b/src/vs/workbench/contrib/void/browser/react/src/void-settings-tsx/MCPServersList.tsx index ef27c58b..a4192a37 100644 --- a/src/vs/workbench/contrib/void/browser/react/src/void-settings-tsx/MCPServersList.tsx +++ b/src/vs/workbench/contrib/void/browser/react/src/void-settings-tsx/MCPServersList.tsx @@ -1,5 +1,5 @@ import { VoidSwitch } from '../util/inputs.js'; -import { MCPServerEventParam, MCPServerObject, MCPServers } from '../../../../common/mcpServiceTypes.js'; +import { MCPConfigParseError, MCPServerEventParam, MCPServerObject, MCPServers } from '../../../../common/mcpServiceTypes.js'; import { useEffect, useState } from 'react'; import { useAccessor } from '../util/services.js'; import { IDisposable } from '../../../../../../../base/common/lifecycle.js'; @@ -102,6 +102,7 @@ const MCPServersList = () => { const accessor = useAccessor(); const mcpService = accessor.get('IMCPService'); const [mcpServers, setMCPServers] = useState({}); + const [mcpConfigError, setMCPConfigError] = useState(null); // Get all servers from MCPConfigService useEffect(() => { @@ -114,35 +115,13 @@ const MCPServersList = () => { setMCPServers(servers); } - const handleListeners = (e: MCPServerEventParam) => { - if (e.response.event === 'add' || e.response.event === 'update' || e.response.event === 'loading') { - // Handle the add event - const { name, newServer } = e.response; - setMCPServers(prevServers => ({ - ...prevServers, - [name]: newServer - })); - return; - } - if (e.response.event === 'delete') { - // Handle the delete event - const { name, prevServer } = e.response; - setMCPServers(prevServers => { - const newServers = { ...prevServers }; - delete newServers[name]; - return newServers; - }); - return; - } - throw new Error('Event not handled'); - } - // Set up listeners for server events const disposables: IDisposable[] = [] disposables.push(mcpService.onDidAddServer(handleListeners)); disposables.push(mcpService.onDidDeleteServer(handleListeners)); disposables.push(mcpService.onDidUpdateServer(handleListeners)); disposables.push(mcpService.onLoadingServers(handleListeners)); + disposables.push(mcpService.onConfigParsingError(handleListeners)); // Clean up subscription when component unmounts return () => { @@ -152,14 +131,48 @@ const MCPServersList = () => { }, [mcpService]); + const handleListeners = (e: MCPServerEventParam | MCPConfigParseError) => { + if (e.response.event === 'config-error') { + // Handle the config error event + const { error } = e.response; + setMCPConfigError(error); + return; + } + if (e.response.event === 'add' || e.response.event === 'update' || e.response.event === 'loading') { + // Handle the add event + const { name, newServer } = e.response; + setMCPServers(prevServers => ({ + ...prevServers, + [name]: newServer + })); + return; + } + if (e.response.event === 'delete') { + // Handle the delete event + const { name, prevServer } = e.response; + setMCPServers(prevServers => { + const newServers = { ...prevServers }; + delete newServers[name]; + return newServers; + }); + return; + } + throw new Error('Event not handled'); + } + return (
- {Object.entries(mcpServers).map(([name, server]) => ( + {!mcpConfigError && Object.entries(mcpServers).map(([name, server]) => (
))} + {mcpConfigError && ( +
+ {mcpConfigError} +
+ )}
); diff --git a/src/vs/workbench/contrib/void/common/mcpService.ts b/src/vs/workbench/contrib/void/common/mcpService.ts index 804abfe8..043a60c5 100644 --- a/src/vs/workbench/contrib/void/common/mcpService.ts +++ b/src/vs/workbench/contrib/void/common/mcpService.ts @@ -15,7 +15,7 @@ import { IProductService } from '../../../../platform/product/common/productServ import { VSBuffer } from '../../../../base/common/buffer.js'; import { IChannel } from '../../../../base/parts/ipc/common/ipc.js'; import { IMainProcessService } from '../../../../platform/ipc/common/mainProcessService.js'; -import { MCPServers, MCPConfig, MCPServerEventParam, MCPServerEventAddParam, MCPServerEventUpdateParam, MCPServerEventDeleteParam, MCPServerEventLoadingParam } from './mcpServiceTypes.js'; +import { MCPServers, MCPConfig, MCPServerEventParam, MCPServerEventAddParam, MCPServerEventUpdateParam, MCPServerEventDeleteParam, MCPServerEventLoadingParam, MCPConfigParseError } from './mcpServiceTypes.js'; import { Event, Emitter } from '../../../../base/common/event.js'; import { InternalToolInfo } from './prompt/prompts.js'; @@ -28,6 +28,7 @@ export interface IMCPService { onDidUpdateServer: Event; onDidDeleteServer: Event; onLoadingServers: Event; + onConfigParsingError: Event; } export const IMCPService = createDecorator('mcpConfigService'); @@ -51,10 +52,12 @@ class MCPService extends Disposable implements IMCPService { private readonly _onDidUpdateServer = new Emitter(); private readonly _onDidDeleteServer = new Emitter(); private readonly _onLoadingServers = new Emitter(); + private readonly _onConfigParsingError = new Emitter(); public readonly onDidAddServer = this._onDidAddServer.event; public readonly onDidUpdateServer = this._onDidUpdateServer.event; public readonly onDidDeleteServer = this._onDidDeleteServer.event; public readonly onLoadingServers = this._onLoadingServers.event; + public readonly onConfigParsingError = this._onConfigParsingError.event; constructor( @IFileService private readonly fileService: IFileService, @@ -110,12 +113,9 @@ class MCPService extends Disposable implements IMCPService { // Parse the MCP config file const mcpConfig = await this._parseMCPConfigFile(); - if (mcpConfig) { - // Create the initial server list - // await this._createInitialServerList(mcpConfig); + if (mcpConfig && mcpConfig.mcpServers) { // Setup the server list - console.log('MCP Config file parsed:', JSON.stringify(mcpConfig, null, 2)); this.channel.call('setupServers', mcpConfig) } @@ -170,14 +170,35 @@ class MCPService extends Disposable implements IMCPService { } private async _parseMCPConfigFile(): Promise { + // Remove any previous config parsing error + // This isn't super intuitive, but it works + this._onConfigParsingError.fire({ + response: { + event: 'config-error', + error: null + } + }); + + // Process config file const mcpConfigUri = await this._getMCPConfigPath(); try { const fileContent = await this.fileService.readFile(mcpConfigUri); const contentString = fileContent.value.toString(); - return JSON.parse(contentString); + const configJson = JSON.parse(contentString); + if (!configJson.mcpServers) { + throw new Error('Invalid MCP config file: missing mcpServers property'); + } + return configJson as MCPConfig; } catch (error) { - console.error('Error reading or parsing MCP config file:', error); + const fullError = `Error parsing MCP config file: ${error}`; + console.error(fullError); + this._onConfigParsingError.fire({ + response: { + event: 'config-error', + error: fullError + } + }); return null; } } @@ -194,7 +215,6 @@ class MCPService extends Disposable implements IMCPService { if (e.contains(mcpConfigUri)) { const mcpConfig = await this._parseMCPConfigFile(); if (mcpConfig && mcpConfig.mcpServers) { - // Call the setupServers method in the main process this.channel.call('setupServers', mcpConfig) } diff --git a/src/vs/workbench/contrib/void/common/mcpServiceTypes.ts b/src/vs/workbench/contrib/void/common/mcpServiceTypes.ts index ec5f728d..0b411bac 100644 --- a/src/vs/workbench/contrib/void/common/mcpServiceTypes.ts +++ b/src/vs/workbench/contrib/void/common/mcpServiceTypes.ts @@ -134,6 +134,14 @@ export interface MCPConfig { mcpServers: Record; } +export interface MCPConfigParseError { + // Error message + response: { + event: 'config-error'; + error: string | null; + } +} + // SERVER EVENT TYPES ------------------------------------------ export interface MCPServerObject {