Skip to content

Commit a74b685

Browse files
dcramerclaude
andauthored
fix: Redact config error details in HTTP transport (#783)
`ConfigurationError`, `LLMProviderError`, and `APICallError` 4xx currently show detailed config guidance to all users and are never reported to Sentry. This is correct for STDIO (the user can fix their own config), but wrong for the hosted Cloudflare deployment where users can't fix server-side config and operators get no visibility into these failures. This adds a `transport` field to `ServerContext` (`"stdio" | "http"`) and makes `formatErrorForUser` transport-aware. In HTTP mode, config/provider errors are now logged to Sentry via `logIssue()` and a generic "Feature Unavailable" message is returned. In STDIO mode (and when transport is unset), the existing detailed messages are preserved unchanged. The transport branching uses a single `formatServerConfigError()` helper that takes the detailed message parts and returns the correct response directly for both transports. `UserInputError` always shows details regardless of transport since the user can fix their own input in both modes. Fixes #781 --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 76f7a9d commit a74b685

6 files changed

Lines changed: 205 additions & 30 deletions

File tree

‎packages/mcp-cloudflare/src/server/lib/mcp-handler.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,7 @@ const mcpHandler: ExportedHandler<Env> = {
173173
mcpUrl: env.MCP_URL,
174174
agentMode: isAgentMode,
175175
experimentalMode: isExperimentalMode,
176+
transport: "http",
176177
};
177178

178179
// Create and configure MCP server with tools filtered by context
Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
import { describe, it, expect, vi, beforeEach } from "vitest";
2+
import { formatErrorForUser } from "./error-handling";
3+
import {
4+
UserInputError,
5+
ConfigurationError,
6+
LLMProviderError,
7+
} from "../errors";
8+
import { APICallError } from "ai";
9+
10+
vi.mock("../telem/logging", () => ({
11+
logIssue: vi.fn(() => "mock-event-id"),
12+
}));
13+
14+
import { logIssue } from "../telem/logging";
15+
16+
describe("formatErrorForUser", () => {
17+
beforeEach(() => {
18+
vi.clearAllMocks();
19+
});
20+
21+
describe("ConfigurationError", () => {
22+
const error = new ConfigurationError("OPENAI_API_KEY is not set");
23+
24+
it("returns detailed message for stdio transport", async () => {
25+
const result = await formatErrorForUser(error, { transport: "stdio" });
26+
expect(result).toContain("OPENAI_API_KEY is not set");
27+
expect(result).toContain("**Configuration Error**");
28+
expect(result).not.toContain("Feature Unavailable");
29+
expect(logIssue).not.toHaveBeenCalled();
30+
});
31+
32+
it("returns generic message for http transport", async () => {
33+
const result = await formatErrorForUser(error, { transport: "http" });
34+
expect(result).toContain("**Feature Unavailable**");
35+
expect(result).not.toContain("OPENAI_API_KEY is not set");
36+
expect(logIssue).toHaveBeenCalledWith(error);
37+
});
38+
39+
it("returns detailed message when transport is undefined (backward compat)", async () => {
40+
const result = await formatErrorForUser(error);
41+
expect(result).toContain("OPENAI_API_KEY is not set");
42+
expect(result).toContain("**Configuration Error**");
43+
expect(logIssue).not.toHaveBeenCalled();
44+
});
45+
});
46+
47+
describe("LLMProviderError", () => {
48+
const error = new LLMProviderError("Region not supported by OpenAI");
49+
50+
it("returns detailed message for stdio transport", async () => {
51+
const result = await formatErrorForUser(error, { transport: "stdio" });
52+
expect(result).toContain("Region not supported by OpenAI");
53+
expect(result).toContain("**AI Provider Error**");
54+
expect(result).not.toContain("Feature Unavailable");
55+
expect(logIssue).not.toHaveBeenCalled();
56+
});
57+
58+
it("returns generic message for http transport", async () => {
59+
const result = await formatErrorForUser(error, { transport: "http" });
60+
expect(result).toContain("**Feature Unavailable**");
61+
expect(result).not.toContain("Region not supported by OpenAI");
62+
expect(logIssue).toHaveBeenCalledWith(error);
63+
});
64+
65+
it("returns detailed message when transport is undefined", async () => {
66+
const result = await formatErrorForUser(error);
67+
expect(result).toContain("Region not supported by OpenAI");
68+
expect(logIssue).not.toHaveBeenCalled();
69+
});
70+
});
71+
72+
describe("APICallError 4xx", () => {
73+
const error = new APICallError({
74+
message: "Invalid API key provided",
75+
url: "https://api.openai.com/v1/chat/completions",
76+
requestBodyValues: {},
77+
statusCode: 401,
78+
isRetryable: false,
79+
});
80+
81+
it("returns detailed message for stdio transport", async () => {
82+
const result = await formatErrorForUser(error, { transport: "stdio" });
83+
expect(result).toContain("Invalid API key provided");
84+
expect(result).toContain("**AI Provider Error**");
85+
expect(result).not.toContain("Feature Unavailable");
86+
expect(logIssue).not.toHaveBeenCalled();
87+
});
88+
89+
it("returns generic message for http transport", async () => {
90+
const result = await formatErrorForUser(error, { transport: "http" });
91+
expect(result).toContain("**Feature Unavailable**");
92+
expect(result).not.toContain("Invalid API key provided");
93+
expect(logIssue).toHaveBeenCalledWith(error);
94+
});
95+
96+
it("returns detailed message when transport is undefined", async () => {
97+
const result = await formatErrorForUser(error);
98+
expect(result).toContain("Invalid API key provided");
99+
expect(logIssue).not.toHaveBeenCalled();
100+
});
101+
});
102+
103+
describe("UserInputError", () => {
104+
const error = new UserInputError("Invalid issue ID format");
105+
106+
it("returns detailed message for http transport (user can fix input)", async () => {
107+
const result = await formatErrorForUser(error, { transport: "http" });
108+
expect(result).toContain("Invalid issue ID format");
109+
expect(result).toContain("**Input Error**");
110+
expect(result).not.toContain("Feature Unavailable");
111+
expect(logIssue).not.toHaveBeenCalled();
112+
});
113+
114+
it("returns detailed message for stdio transport", async () => {
115+
const result = await formatErrorForUser(error, { transport: "stdio" });
116+
expect(result).toContain("Invalid issue ID format");
117+
expect(result).toContain("**Input Error**");
118+
expect(logIssue).not.toHaveBeenCalled();
119+
});
120+
});
121+
});

‎packages/mcp-core/src/internal/error-handling.ts‎

Lines changed: 75 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
import { ApiError, ApiClientError, ApiServerError } from "../api-client";
77
import { logIssue } from "../telem/logging";
88
import { APICallError } from "ai";
9+
import type { TransportType } from "../types";
910

1011
/**
1112
* Type guard to identify user input validation errors.
@@ -58,56 +59,101 @@ export function isAPICallError(error: unknown): error is APICallError {
5859
return APICallError.isInstance(error);
5960
}
6061

62+
const GENERIC_CONFIG_ERROR_MESSAGE = [
63+
"**Feature Unavailable**",
64+
"This feature is temporarily unavailable due to a server configuration issue.",
65+
"The service operator has been notified. Please try again later.",
66+
].join("\n\n");
67+
68+
/**
69+
* Format a server-side configuration error for user display.
70+
*
71+
* For HTTP transport, the detailed message is hidden (the user cannot fix
72+
* server-side configuration) — the error is logged to Sentry and a generic
73+
* message is returned instead.
74+
* For stdio/unset transport, the detailed parts are returned as-is.
75+
*/
76+
function formatServerConfigError(
77+
error: Error,
78+
detailedParts: string[],
79+
options?: { transport?: TransportType },
80+
): string {
81+
if (options?.transport === "http") {
82+
logIssue(error);
83+
return GENERIC_CONFIG_ERROR_MESSAGE;
84+
}
85+
return detailedParts.join("\n\n");
86+
}
87+
6188
/**
6289
* Format an error for user display with markdown formatting.
6390
* This is used by tool handlers to format errors for MCP responses.
6491
*
92+
* When transport is "http", config/provider errors are logged to Sentry
93+
* and a generic message is returned (users can't fix server-side config).
94+
* When transport is "stdio" or undefined, detailed messages are returned
95+
* (users can fix their own config).
96+
*
6597
* SECURITY: Only return trusted error messages to prevent prompt injection vulnerabilities.
6698
* We trust: Sentry API errors, our own UserInputError/ConfigurationError messages, and system templates.
6799
*/
68-
export async function formatErrorForUser(error: unknown): Promise<string> {
100+
export async function formatErrorForUser(
101+
error: unknown,
102+
options?: { transport?: TransportType },
103+
): Promise<string> {
69104
if (isUserInputError(error)) {
70105
return [
71106
"**Input Error**",
72107
"It looks like there was a problem with the input you provided.",
73108
error.message,
74-
`You may be able to resolve the issue by addressing the concern and trying again.`,
109+
"You may be able to resolve the issue by addressing the concern and trying again.",
75110
].join("\n\n");
76111
}
77112

78113
if (isConfigurationError(error)) {
79-
return [
80-
"**Configuration Error**",
81-
"There appears to be a configuration issue with your setup.",
82-
error.message,
83-
`Please check your environment configuration and try again.`,
84-
].join("\n\n");
114+
return formatServerConfigError(
115+
error,
116+
[
117+
"**Configuration Error**",
118+
"There appears to be a configuration issue with your setup.",
119+
error.message,
120+
"Please check your environment configuration and try again.",
121+
],
122+
options,
123+
);
85124
}
86125

87126
if (isLLMProviderError(error)) {
88-
return [
89-
"**AI Provider Error**",
90-
"The AI provider service is not available for this request.",
91-
error.message,
92-
`This is a service availability issue that cannot be resolved by retrying.`,
93-
].join("\n\n");
127+
return formatServerConfigError(
128+
error,
129+
[
130+
"**AI Provider Error**",
131+
"The AI provider service is not available for this request.",
132+
error.message,
133+
"This is a service availability issue that cannot be resolved by retrying.",
134+
],
135+
options,
136+
);
94137
}
95138

96-
// Handle AI SDK APICallError that wasn't converted to LLMProviderError
97-
// This is a defensive layer - ideally callEmbeddedAgent converts these
139+
// Handle AI SDK APICallError that wasn't converted to LLMProviderError.
140+
// This is a defensive layer - ideally callEmbeddedAgent converts these.
98141
if (isAPICallError(error)) {
99142
const statusCode = error.statusCode;
100143
// 4xx errors are user-facing (account issues, rate limits, invalid keys)
101-
// These should NOT be logged to Sentry
102144
if (statusCode && statusCode >= 400 && statusCode < 500) {
103-
return [
104-
"**AI Provider Error**",
105-
"The AI provider service returned an error.",
106-
error.message,
107-
"This may be a configuration or account issue. Please check your AI provider settings.",
108-
].join("\n\n");
145+
return formatServerConfigError(
146+
error,
147+
[
148+
"**AI Provider Error**",
149+
"The AI provider service returned an error.",
150+
error.message,
151+
"This may be a configuration or account issue. Please check your AI provider settings.",
152+
],
153+
options,
154+
);
109155
}
110-
// 5xx errors - log to Sentry
156+
// 5xx errors - always log to Sentry regardless of transport
111157
const eventId = logIssue(error);
112158
const parts = [
113159
"**AI Provider Error**",
@@ -131,7 +177,7 @@ export async function formatErrorForUser(error: unknown): Promise<string> {
131177
"**Input Error**",
132178
statusText,
133179
error.toUserMessage(),
134-
`You may be able to resolve the issue by addressing the concern and trying again.`,
180+
"You may be able to resolve the issue by addressing the concern and trying again.",
135181
].join("\n\n");
136182
}
137183

@@ -142,11 +188,11 @@ export async function formatErrorForUser(error: unknown): Promise<string> {
142188
? `There was an HTTP ${error.status} server error with the Sentry API.`
143189
: "There was a server error.";
144190

145-
const parts = ["**Error**", statusText, `${error.message}`];
191+
const parts = ["**Error**", statusText, error.message];
146192
if (eventId) {
147193
parts.push(`**Event ID**: ${eventId}`);
148194
}
149-
parts.push(`Please contact support if the problem persists.`);
195+
parts.push("Please contact support if the problem persists.");
150196
return parts.join("\n\n");
151197
}
152198

@@ -159,8 +205,8 @@ export async function formatErrorForUser(error: unknown): Promise<string> {
159205
return [
160206
"**Error**",
161207
statusText,
162-
`${error.message}`,
163-
`You may be able to resolve the issue by addressing the concern and trying again.`,
208+
error.message,
209+
"You may be able to resolve the issue by addressing the concern and trying again.",
164210
].join("\n\n");
165211
}
166212

‎packages/mcp-core/src/server.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,9 @@ function configureServer({
373373
content: [
374374
{
375375
type: "text" as const,
376-
text: await formatErrorForUser(error),
376+
text: await formatErrorForUser(error, {
377+
transport: context.transport,
378+
}),
377379
},
378380
],
379381
isError: true,

‎packages/mcp-core/src/types.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ export const CONSTRAINT_PARAMETER_KEYS = new Set<string>([
3838
"regionUrl",
3939
]);
4040

41+
export type TransportType = "stdio" | "http";
42+
4143
export type ServerContext = {
4244
sentryHost?: string;
4345
mcpUrl?: string;
@@ -53,4 +55,6 @@ export type ServerContext = {
5355
agentMode?: boolean;
5456
/** Whether experimental tools are enabled */
5557
experimentalMode?: boolean;
58+
/** Transport type - affects error message formatting */
59+
transport?: TransportType;
5660
};

‎packages/mcp-server/src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,7 @@ const context = {
284284
openaiBaseUrl: cfg.openaiBaseUrl,
285285
agentMode: cli.agent,
286286
experimentalMode: cli.experimental,
287+
transport: "stdio" as const,
287288
};
288289

289290
// Build server with context to filter tools based on granted skills

0 commit comments

Comments
 (0)