Skip to content
Merged
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
1 change: 1 addition & 0 deletions packages/mcp-cloudflare/src/server/lib/mcp-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ const mcpHandler: ExportedHandler<Env> = {
mcpUrl: env.MCP_URL,
agentMode: isAgentMode,
experimentalMode: isExperimentalMode,
transport: "http",
};

// Create and configure MCP server with tools filtered by context
Expand Down
121 changes: 121 additions & 0 deletions packages/mcp-core/src/internal/error-handling.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});
});
});
104 changes: 75 additions & 29 deletions packages/mcp-core/src/internal/error-handling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<string> {
export async function formatErrorForUser(
error: unknown,
options?: { transport?: TransportType },
): Promise<string> {
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**",
Expand All @@ -131,7 +177,7 @@ export async function formatErrorForUser(error: unknown): Promise<string> {
"**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");
}

Expand All @@ -142,11 +188,11 @@ export async function formatErrorForUser(error: unknown): Promise<string> {
? `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");
}

Expand All @@ -159,8 +205,8 @@ export async function formatErrorForUser(error: unknown): Promise<string> {
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");
}

Expand Down
4 changes: 3 additions & 1 deletion packages/mcp-core/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,9 @@ function configureServer({
content: [
{
type: "text" as const,
text: await formatErrorForUser(error),
text: await formatErrorForUser(error, {
transport: context.transport,
}),
},
],
isError: true,
Expand Down
4 changes: 4 additions & 0 deletions packages/mcp-core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ export const CONSTRAINT_PARAMETER_KEYS = new Set<string>([
"regionUrl",
]);

export type TransportType = "stdio" | "http";

export type ServerContext = {
sentryHost?: string;
mcpUrl?: string;
Expand All @@ -53,4 +55,6 @@ export type ServerContext = {
agentMode?: boolean;
/** Whether experimental tools are enabled */
experimentalMode?: boolean;
/** Transport type - affects error message formatting */
transport?: TransportType;
};
1 change: 1 addition & 0 deletions packages/mcp-server/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading