From 34f9e0628a07a1a26c2cbbec1c9ab8525454c889 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 12 Sep 2026 21:09:50 +0000 Subject: [PATCH] fix(mcp): make MCP request handler errors observable The MCP request handler surfaced failures only through createMcpHandler's onerror callback as console.error("MCP request error:", error). The SDK funnels non-Error throws through new Error(String(value)) and Cloudflare's console renders an Error as its stack alone, so the log line carried no message whenever the thrown value was empty or not a well-formed Error. When this error class spiked, operators had a stack with no cause and nothing in Sentry/otel. Add describeThrownError() which renders any thrown value (Error, empty-message Error, non-Error, object, primitive) into a single always-informative line, including status/code and a chained cause where present, and use it in the onerror reporter. The exact 'MCP request error' prefix is preserved so the existing log-based monitoring keeps matching. Co-authored-by: Dhravya Shah --- apps/mcp/src/server/index.ts | 4 +- apps/mcp/src/server/request-error.test.ts | 88 +++++++++++++++++++++++ apps/mcp/src/server/request-error.ts | 80 +++++++++++++++++++++ 3 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 apps/mcp/src/server/request-error.test.ts create mode 100644 apps/mcp/src/server/request-error.ts diff --git a/apps/mcp/src/server/index.ts b/apps/mcp/src/server/index.ts index 70505a21..517b9b10 100644 --- a/apps/mcp/src/server/index.ts +++ b/apps/mcp/src/server/index.ts @@ -10,6 +10,7 @@ import { type AuthUser, } from "./auth" import { SupermemoryMCP } from "./legacy-protocol-state" +import { describeThrownError } from "./request-error" import { createSupermemoryServer } from "./server" import type { ActorContext, ServerEnv } from "./types" import { SpaceState, uploadStateName } from "./space-state" @@ -246,7 +247,8 @@ async function handleMcpRequest( legacy: "stateless", corsOptions: false, allowedOriginHostnames: allowedOriginHostnames(c.env), - onerror: (error) => console.error("MCP request error:", error), + onerror: (error) => + console.error(`MCP request error: ${describeThrownError(error)}`), }, ) diff --git a/apps/mcp/src/server/request-error.test.ts b/apps/mcp/src/server/request-error.test.ts new file mode 100644 index 00000000..c666b66b --- /dev/null +++ b/apps/mcp/src/server/request-error.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, it } from "vitest" +import { describeThrownError } from "./request-error" + +describe("describeThrownError", () => { + it("keeps the name, message, and stack of a well-formed Error", () => { + const error = new Error("something broke") + const described = describeThrownError(error) + + expect(described).toContain("Error: something broke") + expect(described).toContain(error.stack ?? "") + }) + + it("surfaces a placeholder for an empty-message Error", () => { + // The exact shape the incident produced: the SDK reports a stack-only Error + // with no message, so the log line was previously blank. + const error = new Error("") + const described = describeThrownError(error) + + expect(described).toContain("Error: (no message)") + }) + + it("includes the HTTP status and code carried on API-style errors", () => { + const error = Object.assign(new Error("Bad upstream"), { + status: 409, + code: "conflict", + }) + + const described = describeThrownError(error) + + expect(described).toContain("Error: Bad upstream") + expect(described).toContain("status=409") + expect(described).toContain("code=conflict") + }) + + it("preserves a custom error name", () => { + class TransientAuthError extends Error { + override name = "TransientAuthError" + } + + const described = describeThrownError(new TransientAuthError("retry later")) + + expect(described).toContain("TransientAuthError: retry later") + }) + + it("summarizes a chained cause", () => { + const error = new Error("wrapper", { cause: new Error("root reason") }) + + const described = describeThrownError(error) + + expect(described).toContain("Error: wrapper") + expect(described).toContain("caused by Error: root reason") + }) + + it("describes an undefined throw", () => { + expect(describeThrownError(undefined)).toBe( + "non-Error value thrown: undefined", + ) + }) + + it("describes a null throw", () => { + expect(describeThrownError(null)).toBe("non-Error value thrown: null") + }) + + it("describes a thrown string", () => { + expect(describeThrownError("boom")).toBe( + "non-Error value thrown (string): boom", + ) + }) + + it("serializes a thrown plain object", () => { + const described = describeThrownError({ reason: "nope", status: 500 }) + + expect(described).toContain("non-Error value thrown (object):") + expect(described).toContain('"reason":"nope"') + expect(described).toContain('"status":500') + }) + + it("falls back gracefully for a circular object", () => { + const circular: Record = {} + circular.self = circular + + const described = describeThrownError(circular) + + expect(described).toContain("non-Error value thrown (object):") + // Must not throw and must still produce a non-empty description. + expect(described.length).toBeGreaterThan(0) + }) +}) diff --git a/apps/mcp/src/server/request-error.ts b/apps/mcp/src/server/request-error.ts new file mode 100644 index 00000000..e722f55e --- /dev/null +++ b/apps/mcp/src/server/request-error.ts @@ -0,0 +1,80 @@ +// Failures inside the MCP request handler (beyond the auth gate) surface only +// through `createMcpHandler`'s `onerror` callback. The SDK routes every non-Error +// throw through `new Error(String(value))` and Cloudflare's `console.error` +// renders an `Error` as its stack alone, so `console.error("MCP request error:", error)` +// drops the cause entirely when the message is empty and produces a stack with no +// text for anything that wasn't thrown as a well-formed Error. This turns any +// thrown value into a single, always-informative line so the producing failure is +// visible in telemetry instead of being elided. + +function readField(value: object, key: string): unknown { + try { + return Reflect.get(value, key) + } catch { + return undefined + } +} + +function readNumber(value: object, key: string): number | undefined { + const field = readField(value, key) + return typeof field === "number" && Number.isFinite(field) ? field : undefined +} + +function stringifyScalar(value: unknown): string { + if (typeof value === "string") return value + if (typeof value === "bigint") return `${value}n` + return String(value) +} + +function safeStringify(value: object): string { + try { + const json = JSON.stringify(value) + if (json !== undefined && json !== "{}") return json + } catch { + // Fall through to the non-JSON representation (e.g. circular references). + } + try { + return String(value) + } catch { + return "[unserializable value]" + } +} + +// Renders any thrown value into a stable, single description that always carries +// a cause. The exact "MCP request error" prefix stays with the caller so the +// existing log-based monitoring keeps matching; this only fills in the part that +// was previously blank. +export function describeThrownError(error: unknown): string { + if (error instanceof Error) { + const name = error.name || "Error" + const message = error.message || "(no message)" + + const details: string[] = [] + const status = readNumber(error, "status") + if (status !== undefined) details.push(`status=${status}`) + const code = readField(error, "code") + if ( + code !== undefined && + (typeof code === "string" || typeof code === "number") + ) { + details.push(`code=${stringifyScalar(code)}`) + } + const suffix = details.length > 0 ? ` (${details.join(", ")})` : "" + + const cause = error.cause + const causeSuffix = + cause instanceof Error + ? ` caused by ${cause.name || "Error"}: ${cause.message || "(no message)"}` + : "" + + const stack = error.stack ? `\n${error.stack}` : "" + return `${name}: ${message}${suffix}${causeSuffix}${stack}` + } + + if (error === undefined) return "non-Error value thrown: undefined" + if (error === null) return "non-Error value thrown: null" + if (typeof error === "object") { + return `non-Error value thrown (object): ${safeStringify(error)}` + } + return `non-Error value thrown (${typeof error}): ${stringifyScalar(error)}` +}