mirror of
https://github.com/supermemoryai/supermemory.git
synced 2026-10-02 02:11:20 +00:00
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 <dhravya@supermemory.com>
This commit is contained in:
parent
2415a5c796
commit
34f9e0628a
3 changed files with 171 additions and 1 deletions
|
|
@ -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)}`),
|
||||
},
|
||||
)
|
||||
|
||||
|
|
|
|||
88
apps/mcp/src/server/request-error.test.ts
Normal file
88
apps/mcp/src/server/request-error.test.ts
Normal file
|
|
@ -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<string, unknown> = {}
|
||||
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)
|
||||
})
|
||||
})
|
||||
80
apps/mcp/src/server/request-error.ts
Normal file
80
apps/mcp/src/server/request-error.ts
Normal file
|
|
@ -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)}`
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue