fix(cli): guard numeric .code to HTTP range; bound cause-chain depth
Address @claude review of #1583: - statusOf() now only treats a numeric `.code` as an HTTP status when it is in the 100-599 range, so an MCP SDK McpError JSON-RPC code (e.g. -32601 MethodNotFound) is no longer leaked into the envelope `status` or misclassified as AUTH_REQUIRED (it falls back to USAGE). - causeOf() now carries a depth cap (MAX_CAUSE_DEPTH) so a cyclic/ self-referential `error.cause` chain terminates instead of recursing infinitely. - Add tests for both: a -32601 McpError-shaped error (status undefined, exit USAGE) and a self-referential cause chain (finite string). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX
This commit is contained in:
parent
a5d8d80ffc
commit
fa5514a380
2 changed files with 37 additions and 5 deletions
|
|
@ -159,6 +159,18 @@ describe("classifyError", () => {
|
||||||
).toBe("error");
|
).toBe("error");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("terminates on a self-referential cause chain without recursing infinitely", () => {
|
||||||
|
// A cyclic `error.cause` must not hang the classifier; the depth cap bounds
|
||||||
|
// the walk and returns a finite string.
|
||||||
|
const err = new Error("looping");
|
||||||
|
(err as { cause?: unknown }).cause = err;
|
||||||
|
const { envelope } = classifyError(err);
|
||||||
|
expect(envelope.message).toBe("looping");
|
||||||
|
// Bounded by the depth cap: a finite string, not an infinite loop.
|
||||||
|
expect(typeof envelope.cause).toBe("string");
|
||||||
|
expect(envelope.cause).toContain("looping");
|
||||||
|
});
|
||||||
|
|
||||||
it("captures a non-Error cause as a string", () => {
|
it("captures a non-Error cause as a string", () => {
|
||||||
const err = new Error("outer");
|
const err = new Error("outer");
|
||||||
(err as { cause?: unknown }).cause = { reason: "blocked" };
|
(err as { cause?: unknown }).cause = { reason: "blocked" };
|
||||||
|
|
@ -181,6 +193,16 @@ describe("classifyError", () => {
|
||||||
expect(envelope.status).toBe(403);
|
expect(envelope.status).toBe(403);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("ignores a JSON-RPC McpError numeric .code (outside the HTTP range) as a status", () => {
|
||||||
|
// MCP SDK's McpError carries a numeric `.code` that is a JSON-RPC error
|
||||||
|
// code (e.g. -32601 MethodNotFound), NOT an HTTP status. It must not leak
|
||||||
|
// into `status`, nor get misclassified as AUTH_REQUIRED.
|
||||||
|
const err = Object.assign(new Error("Method not found"), { code: -32601 });
|
||||||
|
const { exitCode, envelope } = classifyError(err);
|
||||||
|
expect(exitCode).toBe(EXIT_CODES.USAGE);
|
||||||
|
expect(envelope.status).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
it("ignores a non-numeric node error code when reading the status", () => {
|
it("ignores a non-numeric node error code when reading the status", () => {
|
||||||
const err = Object.assign(new Error("getaddrinfo ENOTFOUND host"), {
|
const err = Object.assign(new Error("getaddrinfo ENOTFOUND host"), {
|
||||||
code: "ENOTFOUND",
|
code: "ENOTFOUND",
|
||||||
|
|
|
||||||
|
|
@ -49,13 +49,18 @@ export class CliExitCodeError extends Error {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Depth cap for {@link causeOf} — bounds a pathological/self-referential
|
||||||
|
* `error.cause` chain so a cycle can never recurse infinitely. */
|
||||||
|
const MAX_CAUSE_DEPTH = 16;
|
||||||
|
|
||||||
/** Read an `Error.cause` chain into a single readable string, if present. */
|
/** Read an `Error.cause` chain into a single readable string, if present. */
|
||||||
function causeOf(error: unknown): string | undefined {
|
function causeOf(error: unknown, depth = 0): string | undefined {
|
||||||
|
if (depth >= MAX_CAUSE_DEPTH) return undefined;
|
||||||
if (!(error instanceof Error)) return undefined;
|
if (!(error instanceof Error)) return undefined;
|
||||||
const cause = (error as { cause?: unknown }).cause;
|
const cause = (error as { cause?: unknown }).cause;
|
||||||
if (cause === undefined || cause === null) return undefined;
|
if (cause === undefined || cause === null) return undefined;
|
||||||
if (cause instanceof Error) {
|
if (cause instanceof Error) {
|
||||||
const nested = causeOf(cause);
|
const nested = causeOf(cause, depth + 1);
|
||||||
return nested ? `${cause.message}: ${nested}` : cause.message;
|
return nested ? `${cause.message}: ${nested}` : cause.message;
|
||||||
}
|
}
|
||||||
return String(cause);
|
return String(cause);
|
||||||
|
|
@ -71,9 +76,14 @@ function statusOf(error: unknown): number | undefined {
|
||||||
};
|
};
|
||||||
if (typeof e.status === "number") return e.status;
|
if (typeof e.status === "number") return e.status;
|
||||||
if (typeof e.response?.status === "number") return e.response.status;
|
if (typeof e.response?.status === "number") return e.response.status;
|
||||||
// SDK SSEError exposes `.code` as the HTTP status; numeric only (avoid
|
// SDK SseError / StreamableHTTPError expose `.code` as the HTTP status. Guard
|
||||||
// mistaking node error codes like "ENOTFOUND" for a status).
|
// to the HTTP range (100-599) so a JSON-RPC McpError code (e.g. -32601
|
||||||
if (typeof e.code === "number") return e.code;
|
// MethodNotFound) is not mistaken for an HTTP status and leaked into the
|
||||||
|
// envelope or misclassified as AUTH_REQUIRED. String node codes like
|
||||||
|
// "ENOTFOUND" are already excluded by the numeric check.
|
||||||
|
if (typeof e.code === "number" && e.code >= 100 && e.code <= 599) {
|
||||||
|
return e.code;
|
||||||
|
}
|
||||||
return undefined;
|
return undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue