Skip to content

Commit 47747d6

Browse files
author
aheizi
committed
validate server config
1 parent e2182eb commit 47747d6

File tree

1 file changed

+177
-72
lines changed

1 file changed

+177
-72
lines changed

‎src/services/mcp/McpHub.ts‎

Lines changed: 177 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -43,26 +43,54 @@ const BaseConfigSchema = z.object({
4343
alwaysAllow: z.array(z.string()).default([]),
4444
})
4545

46-
// Server configuration schema with automatic type inference
47-
export const ServerConfigSchema = z.union([
48-
// Stdio config (has command field)
49-
BaseConfigSchema.extend({
50-
command: z.string(),
51-
args: z.array(z.string()).optional(),
52-
env: z.record(z.string()).optional(),
53-
}).transform((data) => ({
54-
...data,
55-
type: "stdio" as const,
56-
})),
57-
// SSE config (has url field)
58-
BaseConfigSchema.extend({
59-
url: z.string().url(),
60-
headers: z.record(z.string()).optional(),
61-
}).transform((data) => ({
62-
...data,
63-
type: "sse" as const,
64-
})),
65-
])
46+
// Custom error messages for better user feedback
47+
const typeErrorMessage = "Server type must be either 'stdio' or 'sse'"
48+
const stdioFieldsErrorMessage =
49+
"For 'stdio' type servers, you must provide a 'command' field and can optionally include 'args' and 'env'"
50+
const sseFieldsErrorMessage =
51+
"For 'sse' type servers, you must provide a 'url' field and can optionally include 'headers'"
52+
const mixedFieldsErrorMessage =
53+
"Cannot mix 'stdio' and 'sse' fields. For 'stdio' use 'command', 'args', and 'env'. For 'sse' use 'url' and 'headers'"
54+
const missingFieldsErrorMessage = "Server configuration must include either 'command' (for stdio) or 'url' (for sse)"
55+
56+
// Helper function to create a refined schema with better error messages
57+
const createServerTypeSchema = () => {
58+
return z.union([
59+
// Stdio config (has command field)
60+
BaseConfigSchema.extend({
61+
type: z.enum(["stdio"]).optional(),
62+
command: z.string().min(1, "Command cannot be empty"),
63+
args: z.array(z.string()).optional(),
64+
env: z.record(z.string()).optional(),
65+
// Ensure no SSE fields are present
66+
url: z.undefined().optional(),
67+
headers: z.undefined().optional(),
68+
})
69+
.transform((data) => ({
70+
...data,
71+
type: "stdio" as const,
72+
}))
73+
.refine((data) => data.type === undefined || data.type === "stdio", { message: typeErrorMessage }),
74+
// SSE config (has url field)
75+
BaseConfigSchema.extend({
76+
type: z.enum(["sse"]).optional(),
77+
url: z.string().url("URL must be a valid URL format"),
78+
headers: z.record(z.string()).optional(),
79+
// Ensure no stdio fields are present
80+
command: z.undefined().optional(),
81+
args: z.undefined().optional(),
82+
env: z.undefined().optional(),
83+
})
84+
.transform((data) => ({
85+
...data,
86+
type: "sse" as const,
87+
}))
88+
.refine((data) => data.type === undefined || data.type === "sse", { message: typeErrorMessage }),
89+
])
90+
}
91+
92+
// Server configuration schema with automatic type inference and validation
93+
export const ServerConfigSchema = createServerTypeSchema()
6694

6795
// Settings schema
6896
const McpSettingsSchema = z.object({
@@ -84,6 +112,74 @@ export class McpHub {
84112
this.initializeMcpServers()
85113
}
86114

115+
/**
116+
* Validates and normalizes server configuration
117+
* @param config The server configuration to validate
118+
* @param serverName Optional server name for error messages
119+
* @returns The validated configuration
120+
* @throws Error if the configuration is invalid
121+
*/
122+
private validateServerConfig(config: any, serverName?: string): z.infer<typeof ServerConfigSchema> {
123+
// Detect configuration issues before validation
124+
const hasStdioFields = config.command !== undefined
125+
const hasSseFields = config.url !== undefined
126+
127+
// Check for mixed fields
128+
if (hasStdioFields && hasSseFields) {
129+
throw new Error(mixedFieldsErrorMessage)
130+
}
131+
132+
// Check if it's a stdio or SSE config and add type if missing
133+
if (!config.type) {
134+
if (hasStdioFields) {
135+
config.type = "stdio"
136+
} else if (hasSseFields) {
137+
config.type = "sse"
138+
} else {
139+
throw new Error(missingFieldsErrorMessage)
140+
}
141+
} else if (config.type !== "stdio" && config.type !== "sse") {
142+
throw new Error(typeErrorMessage)
143+
}
144+
145+
// Check for type/field mismatch
146+
if (config.type === "stdio" && !hasStdioFields) {
147+
throw new Error(stdioFieldsErrorMessage)
148+
}
149+
if (config.type === "sse" && !hasSseFields) {
150+
throw new Error(sseFieldsErrorMessage)
151+
}
152+
153+
// Validate the config against the schema
154+
try {
155+
return ServerConfigSchema.parse(config)
156+
} catch (validationError) {
157+
if (validationError instanceof z.ZodError) {
158+
// Extract and format validation errors
159+
const errorMessages = validationError.errors
160+
.map((err) => `${err.path.join(".")}: ${err.message}`)
161+
.join("; ")
162+
throw new Error(
163+
serverName
164+
? `Invalid configuration for server "${serverName}": ${errorMessages}`
165+
: `Invalid server configuration: ${errorMessages}`,
166+
)
167+
}
168+
throw validationError
169+
}
170+
}
171+
172+
/**
173+
* Formats and displays error messages to the user
174+
* @param message The error message prefix
175+
* @param error The error object
176+
*/
177+
private showErrorMessage(message: string, error: unknown): void {
178+
const errorMessage = error instanceof Error ? error.message : `${error}`
179+
console.error(`${message}:`, error)
180+
vscode.window.showErrorMessage(`${message}: ${errorMessage}`)
181+
}
182+
87183
getServers(): McpServer[] {
88184
// Only return enabled servers
89185
return this.connections.filter((conn) => !conn.server.disabled).map((conn) => conn.server)
@@ -133,7 +229,7 @@ export class McpHub {
133229
if (arePathsEqual(document.uri.fsPath, settingsPath)) {
134230
const content = await fs.readFile(settingsPath, "utf-8")
135231
const errorMessage =
136-
"Invalid MCP settings format. Please ensure your settings follow the correct JSON format."
232+
"Invalid MCP settings JSON format. Please ensure your settings follow the correct JSON format."
137233
let config: any
138234
try {
139235
config = JSON.parse(content)
@@ -143,13 +239,16 @@ export class McpHub {
143239
}
144240
const result = McpSettingsSchema.safeParse(config)
145241
if (!result.success) {
146-
vscode.window.showErrorMessage(errorMessage)
242+
const errorMessages = result.error.errors
243+
.map((err) => `${err.path.join(".")}: ${err.message}`)
244+
.join("\n")
245+
vscode.window.showErrorMessage(`Invalid MCP settings format: ${errorMessages}`)
147246
return
148247
}
149248
try {
150249
await this.updateServerConnections(result.data.mcpServers || {})
151250
} catch (error) {
152-
console.error("Failed to process MCP settings change:", error)
251+
this.showErrorMessage("Failed to process MCP settings change", error)
153252
}
154253
}
155254
}),
@@ -160,19 +259,39 @@ export class McpHub {
160259
try {
161260
const settingsPath = await this.getMcpSettingsFilePath()
162261
const content = await fs.readFile(settingsPath, "utf-8")
163-
const config = JSON.parse(content)
262+
let config: any
263+
264+
try {
265+
config = JSON.parse(content)
266+
} catch (parseError) {
267+
const errorMessage =
268+
"Invalid MCP settings JSON format. Please check your settings file for syntax errors."
269+
console.error(errorMessage, parseError)
270+
vscode.window.showErrorMessage(errorMessage)
271+
return
272+
}
164273

165274
// Validate the config using McpSettingsSchema
166275
const result = McpSettingsSchema.safeParse(config)
167276
if (result.success) {
168277
await this.updateServerConnections(result.data.mcpServers || {})
169278
} else {
170-
console.error("Invalid MCP settings format:", result.error)
171-
// Still try to connect with the raw config
172-
await this.updateServerConnections(config.mcpServers || {})
279+
// Format validation errors for better user feedback
280+
const errorMessages = result.error.errors
281+
.map((err) => `${err.path.join(".")}: ${err.message}`)
282+
.join("\n")
283+
console.error("Invalid MCP settings format:", errorMessages)
284+
vscode.window.showErrorMessage(`Invalid MCP settings format: ${errorMessages}`)
285+
286+
// Still try to connect with the raw config, but show warnings
287+
try {
288+
await this.updateServerConnections(config.mcpServers || {})
289+
} catch (error) {
290+
this.showErrorMessage("Failed to initialize MCP servers with raw config", error)
291+
}
173292
}
174293
} catch (error) {
175-
console.error("Failed to initialize MCP servers:", error)
294+
this.showErrorMessage("Failed to initialize MCP servers", error)
176295
}
177296
}
178297

@@ -210,7 +329,7 @@ export class McpHub {
210329
const connection = this.connections.find((conn) => conn.server.name === name)
211330
if (connection) {
212331
connection.server.status = "disconnected"
213-
this.appendErrorMessage(connection, error.message)
332+
this.appendErrorMessage(connection, error instanceof Error ? error.message : `${error}`)
214333
}
215334
await this.notifyWebviewOfServerChanges()
216335
}
@@ -269,7 +388,7 @@ export class McpHub {
269388
const connection = this.connections.find((conn) => conn.server.name === name)
270389
if (connection) {
271390
connection.server.status = "disconnected"
272-
this.appendErrorMessage(connection, error.message)
391+
this.appendErrorMessage(connection, error instanceof Error ? error.message : `${error}`)
273392
}
274393
await this.notifyWebviewOfServerChanges()
275394
}
@@ -301,15 +420,20 @@ export class McpHub {
301420
const connection = this.connections.find((conn) => conn.server.name === name)
302421
if (connection) {
303422
connection.server.status = "disconnected"
304-
this.appendErrorMessage(connection, error instanceof Error ? error.message : String(error))
423+
this.appendErrorMessage(connection, error instanceof Error ? error.message : `${error}`)
305424
}
306425
throw error
307426
}
308427
}
309428

310429
private appendErrorMessage(connection: McpConnection, error: string) {
430+
// Limit error message length to prevent excessive length
431+
const maxErrorLength = 1000
311432
const newError = connection.server.error ? `${connection.server.error}\n${error}` : error
312-
connection.server.error = newError //.slice(0, 800)
433+
connection.server.error =
434+
newError.length > maxErrorLength
435+
? newError.substring(0, maxErrorLength) + "...(error message truncated)"
436+
: newError
313437
}
314438

315439
private async fetchToolsList(serverName: string): Promise<McpTool[]> {
@@ -396,17 +520,9 @@ export class McpHub {
396520
// Validate and transform the config
397521
let validatedConfig: z.infer<typeof ServerConfigSchema>
398522
try {
399-
// Check if it's a stdio or SSE config and add type if missing
400-
if (!config.type) {
401-
if (config.command) {
402-
config.type = "stdio"
403-
} else if (config.url) {
404-
config.type = "sse"
405-
}
406-
}
407-
validatedConfig = ServerConfigSchema.parse(config)
523+
validatedConfig = this.validateServerConfig(config, name)
408524
} catch (error) {
409-
console.error(`Invalid configuration for MCP server ${name}:`, error)
525+
this.showErrorMessage(`Invalid configuration for MCP server "${name}"`, error)
410526
continue
411527
}
412528

@@ -416,7 +532,7 @@ export class McpHub {
416532
this.setupFileWatcher(name, validatedConfig)
417533
await this.connectToServer(name, validatedConfig)
418534
} catch (error) {
419-
console.error(`Failed to connect to new MCP server ${name}:`, error)
535+
this.showErrorMessage(`Failed to connect to new MCP server ${name}`, error)
420536
}
421537
} else if (!deepEqual(JSON.parse(currentConnection.server.config), config)) {
422538
// Existing server with changed config
@@ -426,7 +542,7 @@ export class McpHub {
426542
await this.connectToServer(name, validatedConfig)
427543
console.log(`Reconnected MCP server with updated config: ${name}`)
428544
} catch (error) {
429-
console.error(`Failed to reconnect MCP server ${name}:`, error)
545+
this.showErrorMessage(`Failed to reconnect MCP server ${name}`, error)
430546
}
431547
}
432548
// If server exists with same config, do nothing
@@ -480,12 +596,20 @@ export class McpHub {
480596
await delay(500) // artificial delay to show user that server is restarting
481597
try {
482598
await this.deleteConnection(serverName)
483-
// Try to connect again using existing config
484-
await this.connectToServer(serverName, JSON.parse(config))
485-
vscode.window.showInformationMessage(`${serverName} MCP server connected`)
599+
// Parse the config to validate it
600+
const parsedConfig = JSON.parse(config)
601+
try {
602+
// Validate the config
603+
const validatedConfig = this.validateServerConfig(parsedConfig, serverName)
604+
605+
// Try to connect again using validated config
606+
await this.connectToServer(serverName, validatedConfig)
607+
vscode.window.showInformationMessage(`${serverName} MCP server connected`)
608+
} catch (validationError) {
609+
this.showErrorMessage(`Invalid configuration for MCP server "${serverName}"`, validationError)
610+
}
486611
} catch (error) {
487-
console.error(`Failed to restart connection for ${serverName}:`, error)
488-
vscode.window.showErrorMessage(`Failed to connect to ${serverName} MCP server`)
612+
this.showErrorMessage(`Failed to restart ${serverName} MCP server connection`, error)
489613
}
490614
}
491615

@@ -575,13 +699,7 @@ export class McpHub {
575699
await this.notifyWebviewOfServerChanges()
576700
}
577701
} catch (error) {
578-
console.error("Failed to update server disabled state:", error)
579-
if (error instanceof Error) {
580-
console.error("Error details:", error.message, error.stack)
581-
}
582-
vscode.window.showErrorMessage(
583-
`Failed to update server state: ${error instanceof Error ? error.message : String(error)}`,
584-
)
702+
this.showErrorMessage(`Failed to update server ${serverName} state`, error)
585703
throw error
586704
}
587705
}
@@ -628,13 +746,7 @@ export class McpHub {
628746
await this.notifyWebviewOfServerChanges()
629747
}
630748
} catch (error) {
631-
console.error("Failed to update server timeout:", error)
632-
if (error instanceof Error) {
633-
console.error("Error details:", error.message, error.stack)
634-
}
635-
vscode.window.showErrorMessage(
636-
`Failed to update server timeout: ${error instanceof Error ? error.message : String(error)}`,
637-
)
749+
this.showErrorMessage(`Failed to update server ${serverName} timeout settings`, error)
638750
throw error
639751
}
640752
}
@@ -681,13 +793,7 @@ export class McpHub {
681793
vscode.window.showWarningMessage(`Server "${serverName}" not found in configuration`)
682794
}
683795
} catch (error) {
684-
console.error("Failed to delete MCP server:", error)
685-
if (error instanceof Error) {
686-
console.error("Error details:", error.message, error.stack)
687-
}
688-
vscode.window.showErrorMessage(
689-
`Failed to delete MCP server: ${error instanceof Error ? error.message : String(error)}`,
690-
)
796+
this.showErrorMessage(`Failed to delete MCP server ${serverName}`, error)
691797
throw error
692798
}
693799
}
@@ -783,8 +889,7 @@ export class McpHub {
783889
await this.notifyWebviewOfServerChanges()
784890
}
785891
} catch (error) {
786-
console.error("Failed to update always allow settings:", error)
787-
vscode.window.showErrorMessage("Failed to update always allow settings")
892+
this.showErrorMessage(`Failed to update always allow settings for tool ${toolName}`, error)
788893
throw error // Re-throw to ensure the error is properly handled
789894
}
790895
}

0 commit comments

Comments
 (0)