diff --git a/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts b/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts index af7078dc0..d3cd1a0b2 100644 --- a/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts +++ b/packages/mcp-cloudflare/src/server/lib/mcp-handler.ts @@ -173,6 +173,7 @@ const mcpHandler: ExportedHandler = { mcpUrl: env.MCP_URL, agentMode: isAgentMode, experimentalMode: isExperimentalMode, + transport: "http", }; // Create and configure MCP server with tools filtered by context diff --git a/packages/mcp-core/src/internal/error-handling.test.ts b/packages/mcp-core/src/internal/error-handling.test.ts new file mode 100644 index 000000000..d1f2282d4 --- /dev/null +++ b/packages/mcp-core/src/internal/error-handling.test.ts @@ -0,0 +1,121 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { formatErrorForUser } from "./error-handling"; +import { + UserInputError, + ConfigurationError, + LLMProviderError, +} from "../errors"; +import { APICallError } from "ai"; + +vi.mock("../telem/logging", () => ({ + logIssue: vi.fn(() => "mock-event-id"), +})); + +import { logIssue } from "../telem/logging"; + +describe("formatErrorForUser", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe("ConfigurationError", () => { + const error = new ConfigurationError("OPENAI_API_KEY is not set"); + + it("returns detailed message for stdio transport", async () => { + const result = await formatErrorForUser(error, { transport: "stdio" }); + expect(result).toContain("OPENAI_API_KEY is not set"); + expect(result).toContain("**Configuration Error**"); + expect(result).not.toContain("Feature Unavailable"); + expect(logIssue).not.toHaveBeenCalled(); + }); + + it("returns generic message for http transport", async () => { + const result = await formatErrorForUser(error, { transport: "http" }); + expect(result).toContain("**Feature Unavailable**"); + expect(result).not.toContain("OPENAI_API_KEY is not set"); + expect(logIssue).toHaveBeenCalledWith(error); + }); + + it("returns detailed message when transport is undefined (backward compat)", async () => { + const result = await formatErrorForUser(error); + expect(result).toContain("OPENAI_API_KEY is not set"); + expect(result).toContain("**Configuration Error**"); + expect(logIssue).not.toHaveBeenCalled(); + }); + }); + + describe("LLMProviderError", () => { + const error = new LLMProviderError("Region not supported by OpenAI"); + + it("returns detailed message for stdio transport", async () => { + const result = await formatErrorForUser(error, { transport: "stdio" }); + expect(result).toContain("Region not supported by OpenAI"); + expect(result).toContain("**AI Provider Error**"); + expect(result).not.toContain("Feature Unavailable"); + expect(logIssue).not.toHaveBeenCalled(); + }); + + it("returns generic message for http transport", async () => { + const result = await formatErrorForUser(error, { transport: "http" }); + expect(result).toContain("**Feature Unavailable**"); + expect(result).not.toContain("Region not supported by OpenAI"); + expect(logIssue).toHaveBeenCalledWith(error); + }); + + it("returns detailed message when transport is undefined", async () => { + const result = await formatErrorForUser(error); + expect(result).toContain("Region not supported by OpenAI"); + expect(logIssue).not.toHaveBeenCalled(); + }); + }); + + describe("APICallError 4xx", () => { + const error = new APICallError({ + message: "Invalid API key provided", + url: "https://api.openai.com/v1/chat/completions", + requestBodyValues: {}, + statusCode: 401, + isRetryable: false, + }); + + it("returns detailed message for stdio transport", async () => { + const result = await formatErrorForUser(error, { transport: "stdio" }); + expect(result).toContain("Invalid API key provided"); + expect(result).toContain("**AI Provider Error**"); + expect(result).not.toContain("Feature Unavailable"); + expect(logIssue).not.toHaveBeenCalled(); + }); + + it("returns generic message for http transport", async () => { + const result = await formatErrorForUser(error, { transport: "http" }); + expect(result).toContain("**Feature Unavailable**"); + expect(result).not.toContain("Invalid API key provided"); + expect(logIssue).toHaveBeenCalledWith(error); + }); + + it("returns detailed message when transport is undefined", async () => { + const result = await formatErrorForUser(error); + expect(result).toContain("Invalid API key provided"); + expect(logIssue).not.toHaveBeenCalled(); + }); + }); + + describe("UserInputError", () => { + const error = new UserInputError("Invalid issue ID format"); + + it("returns detailed message for http transport (user can fix input)", async () => { + const result = await formatErrorForUser(error, { transport: "http" }); + expect(result).toContain("Invalid issue ID format"); + expect(result).toContain("**Input Error**"); + expect(result).not.toContain("Feature Unavailable"); + expect(logIssue).not.toHaveBeenCalled(); + }); + + it("returns detailed message for stdio transport", async () => { + const result = await formatErrorForUser(error, { transport: "stdio" }); + expect(result).toContain("Invalid issue ID format"); + expect(result).toContain("**Input Error**"); + expect(logIssue).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/mcp-core/src/internal/error-handling.ts b/packages/mcp-core/src/internal/error-handling.ts index 003307574..8e4b9b79e 100644 --- a/packages/mcp-core/src/internal/error-handling.ts +++ b/packages/mcp-core/src/internal/error-handling.ts @@ -6,6 +6,7 @@ import { import { ApiError, ApiClientError, ApiServerError } from "../api-client"; import { logIssue } from "../telem/logging"; import { APICallError } from "ai"; +import type { TransportType } from "../types"; /** * Type guard to identify user input validation errors. @@ -58,56 +59,101 @@ export function isAPICallError(error: unknown): error is APICallError { return APICallError.isInstance(error); } +const GENERIC_CONFIG_ERROR_MESSAGE = [ + "**Feature Unavailable**", + "This feature is temporarily unavailable due to a server configuration issue.", + "The service operator has been notified. Please try again later.", +].join("\n\n"); + +/** + * Format a server-side configuration error for user display. + * + * For HTTP transport, the detailed message is hidden (the user cannot fix + * server-side configuration) — the error is logged to Sentry and a generic + * message is returned instead. + * For stdio/unset transport, the detailed parts are returned as-is. + */ +function formatServerConfigError( + error: Error, + detailedParts: string[], + options?: { transport?: TransportType }, +): string { + if (options?.transport === "http") { + logIssue(error); + return GENERIC_CONFIG_ERROR_MESSAGE; + } + return detailedParts.join("\n\n"); +} + /** * Format an error for user display with markdown formatting. * This is used by tool handlers to format errors for MCP responses. * + * When transport is "http", config/provider errors are logged to Sentry + * and a generic message is returned (users can't fix server-side config). + * When transport is "stdio" or undefined, detailed messages are returned + * (users can fix their own config). + * * SECURITY: Only return trusted error messages to prevent prompt injection vulnerabilities. * We trust: Sentry API errors, our own UserInputError/ConfigurationError messages, and system templates. */ -export async function formatErrorForUser(error: unknown): Promise { +export async function formatErrorForUser( + error: unknown, + options?: { transport?: TransportType }, +): Promise { if (isUserInputError(error)) { return [ "**Input Error**", "It looks like there was a problem with the input you provided.", error.message, - `You may be able to resolve the issue by addressing the concern and trying again.`, + "You may be able to resolve the issue by addressing the concern and trying again.", ].join("\n\n"); } if (isConfigurationError(error)) { - return [ - "**Configuration Error**", - "There appears to be a configuration issue with your setup.", - error.message, - `Please check your environment configuration and try again.`, - ].join("\n\n"); + return formatServerConfigError( + error, + [ + "**Configuration Error**", + "There appears to be a configuration issue with your setup.", + error.message, + "Please check your environment configuration and try again.", + ], + options, + ); } if (isLLMProviderError(error)) { - return [ - "**AI Provider Error**", - "The AI provider service is not available for this request.", - error.message, - `This is a service availability issue that cannot be resolved by retrying.`, - ].join("\n\n"); + return formatServerConfigError( + error, + [ + "**AI Provider Error**", + "The AI provider service is not available for this request.", + error.message, + "This is a service availability issue that cannot be resolved by retrying.", + ], + options, + ); } - // Handle AI SDK APICallError that wasn't converted to LLMProviderError - // This is a defensive layer - ideally callEmbeddedAgent converts these + // Handle AI SDK APICallError that wasn't converted to LLMProviderError. + // This is a defensive layer - ideally callEmbeddedAgent converts these. if (isAPICallError(error)) { const statusCode = error.statusCode; // 4xx errors are user-facing (account issues, rate limits, invalid keys) - // These should NOT be logged to Sentry if (statusCode && statusCode >= 400 && statusCode < 500) { - return [ - "**AI Provider Error**", - "The AI provider service returned an error.", - error.message, - "This may be a configuration or account issue. Please check your AI provider settings.", - ].join("\n\n"); + return formatServerConfigError( + error, + [ + "**AI Provider Error**", + "The AI provider service returned an error.", + error.message, + "This may be a configuration or account issue. Please check your AI provider settings.", + ], + options, + ); } - // 5xx errors - log to Sentry + // 5xx errors - always log to Sentry regardless of transport const eventId = logIssue(error); const parts = [ "**AI Provider Error**", @@ -131,7 +177,7 @@ export async function formatErrorForUser(error: unknown): Promise { "**Input Error**", statusText, error.toUserMessage(), - `You may be able to resolve the issue by addressing the concern and trying again.`, + "You may be able to resolve the issue by addressing the concern and trying again.", ].join("\n\n"); } @@ -142,11 +188,11 @@ export async function formatErrorForUser(error: unknown): Promise { ? `There was an HTTP ${error.status} server error with the Sentry API.` : "There was a server error."; - const parts = ["**Error**", statusText, `${error.message}`]; + const parts = ["**Error**", statusText, error.message]; if (eventId) { parts.push(`**Event ID**: ${eventId}`); } - parts.push(`Please contact support if the problem persists.`); + parts.push("Please contact support if the problem persists."); return parts.join("\n\n"); } @@ -159,8 +205,8 @@ export async function formatErrorForUser(error: unknown): Promise { return [ "**Error**", statusText, - `${error.message}`, - `You may be able to resolve the issue by addressing the concern and trying again.`, + error.message, + "You may be able to resolve the issue by addressing the concern and trying again.", ].join("\n\n"); } diff --git a/packages/mcp-core/src/server.ts b/packages/mcp-core/src/server.ts index 9d2164284..841207ef5 100644 --- a/packages/mcp-core/src/server.ts +++ b/packages/mcp-core/src/server.ts @@ -373,7 +373,9 @@ function configureServer({ content: [ { type: "text" as const, - text: await formatErrorForUser(error), + text: await formatErrorForUser(error, { + transport: context.transport, + }), }, ], isError: true, diff --git a/packages/mcp-core/src/types.ts b/packages/mcp-core/src/types.ts index 34cfad2a0..a2cf6f1fd 100644 --- a/packages/mcp-core/src/types.ts +++ b/packages/mcp-core/src/types.ts @@ -38,6 +38,8 @@ export const CONSTRAINT_PARAMETER_KEYS = new Set([ "regionUrl", ]); +export type TransportType = "stdio" | "http"; + export type ServerContext = { sentryHost?: string; mcpUrl?: string; @@ -53,4 +55,6 @@ export type ServerContext = { agentMode?: boolean; /** Whether experimental tools are enabled */ experimentalMode?: boolean; + /** Transport type - affects error message formatting */ + transport?: TransportType; }; diff --git a/packages/mcp-server/src/index.ts b/packages/mcp-server/src/index.ts index 7a771d60f..049edd149 100644 --- a/packages/mcp-server/src/index.ts +++ b/packages/mcp-server/src/index.ts @@ -284,6 +284,7 @@ const context = { openaiBaseUrl: cfg.openaiBaseUrl, agentMode: cli.agent, experimentalMode: cli.experimental, + transport: "stdio" as const, }; // Build server with context to filter tools based on granted skills