From 0c440bfc07f3cb343e2e836d27cccaed539cfe9c Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Tue, 20 Oct 2026 19:54:56 +0800 Subject: [PATCH] fix(cli): match listener recovery guidance to system failure Classify listener errno and lifecycle phase at the transport owner. Preserve native cause and unknown codes while distinguishing host lookup, unavailable address, occupied port and permission recovery. Refs #5455. --- src/cli/environmentCommands.ts | 50 ++++- src/server/environment/serveEnvironment.ts | 3 +- src/server/http/serveStreamableHttp.ts | 6 +- src/server/http/streamableHttpListenError.ts | 79 +++++++- .../cli/environmentServeRemediation.test.ts | 179 ++++++++++++++++++ 5 files changed, 302 insertions(+), 15 deletions(-) create mode 100644 tests/boundary/cli/environmentServeRemediation.test.ts diff --git a/src/cli/environmentCommands.ts b/src/cli/environmentCommands.ts index c30d1c11d6..7a95e0b835 100644 --- a/src/cli/environmentCommands.ts +++ b/src/cli/environmentCommands.ts @@ -12,6 +12,7 @@ import { type McpHttpEndpoint, } from "../config/mcpHttpEndpoint.js"; import type { AnalysisError } from "../domain/analysisErrorBase.js"; +import type { StreamableHttpListenError } from "../server/http/streamableHttpListenError.js"; import { AnalysisCancelledError, AnalysisResourceConstraintError, @@ -47,10 +48,10 @@ const untilAborted = (signal: AbortSignal): Promise => }); }); -/** A listener that could not bind or failed after binding, as the command's typed refusal. */ -const transportError = ( +/** A listener failure as the command's typed refusal. */ +export const transportError = ( endpoint: McpHttpEndpoint, - cause: Error, + cause: StreamableHttpListenError, ): AnalysisResourceConstraintError => new AnalysisResourceConstraintError( CLI_COMMANDS.environmentServe, @@ -60,11 +61,41 @@ const transportError = ( { cause, retryable: false, - remediationAction: - "Choose another --port or MCP_PORT, or stop the process using it.", + diagnostics: { + listener_failure: cause.classification.ok + ? { + kind: cause.classification.value.kind, + system_code: cause.classification.value.systemCode, + } + : { + kind: cause.classification.error.kind, + ...(cause.classification.error.systemCode === undefined + ? {} + : { system_code: cause.classification.error.systemCode }), + }, + listener_phase: cause.phase, + }, + remediationAction: listenerRemediation(cause.classification), }, ); +const listenerRemediation = ( + classification: StreamableHttpListenError["classification"], +): string => { + if (!classification.ok) + return "Check the selected listener host, port, and binding permissions, then retry."; + switch (classification.value.kind) { + case "host_lookup": + return "Check --host or MCP_HOST and host name resolution; correct the value or resolve the lookup failure, then retry."; + case "host_unavailable": + return "Choose an address assigned to this machine with --host or MCP_HOST, then retry."; + case "port_in_use": + return "Choose another --port or MCP_PORT, or stop the process using it."; + case "permission_denied": + return "Choose a host and port this process is permitted to bind; if the selected port is restricted, use an allowed port."; + } +}; + interface ServeArguments { readonly manifest: string; readonly evidence: string; @@ -116,8 +147,9 @@ const serveEnvironmentCommand = async ( await import("../server/http/streamableHttpListenError.js"); if (signal.aborted) return cancelled(); // A listener that fails after binding ends the command instead of serving nothing. - let failListener: (error: Error) => void = () => undefined; - const listenerFailed = new Promise((resolve) => { + let failListener: (error: StreamableHttpListenError) => void = () => + undefined; + const listenerFailed = new Promise((resolve) => { failListener = resolve; }); let served: import("../server/environment/serveEnvironment.js").ServedEnvironment; @@ -130,7 +162,7 @@ const serveEnvironmentCommand = async ( ? {} : { environmentName: environmentName.value }), logger: listenerLogger(environment, logger), - onerror: (error) => failListener(error), + onerror: failListener, }); if (!started.ok) return cliError(started.error); served = started.value; @@ -139,7 +171,7 @@ const serveEnvironmentCommand = async ( if (!(cause instanceof StreamableHttpListenError)) throw cause; return cliError(transportError(endpoint.value, cause)); } - let failure: Error | undefined; + let failure: StreamableHttpListenError | undefined; try { failure = await Promise.race([untilAborted(signal), listenerFailed]); } finally { diff --git a/src/server/environment/serveEnvironment.ts b/src/server/environment/serveEnvironment.ts index d3e12f48bd..46de92dae0 100644 --- a/src/server/environment/serveEnvironment.ts +++ b/src/server/environment/serveEnvironment.ts @@ -23,6 +23,7 @@ import { projectAnalysisError } from "../../domain/analysisErrorProjection.js"; import { isExactJsonNumber } from "../../domain/exactJsonNumber.js"; import { safeParseJson } from "../../domain/safeJson.js"; import type { AnalysisResourceConstraintError } from "../../domain/analysisErrorCore.js"; +import type { StreamableHttpListenError } from "../http/streamableHttpListenError.js"; import { ok, type Result } from "../../domain/result.js"; import { PRODUCT_IDENTITY } from "../../identity.js"; import { @@ -212,7 +213,7 @@ export interface ServeEnvironmentOptions { readonly environmentName?: string; readonly logger?: Logger; /** Reports a listener failure after binding; the failure is logged either way. */ - readonly onerror?: (error: Error) => void; + readonly onerror?: (error: StreamableHttpListenError) => void; } /** A listening agent environment and the card it serves. */ diff --git a/src/server/http/serveStreamableHttp.ts b/src/server/http/serveStreamableHttp.ts index 56c0fde8cc..f953ffe504 100644 --- a/src/server/http/serveStreamableHttp.ts +++ b/src/server/http/serveStreamableHttp.ts @@ -60,7 +60,7 @@ export interface ServeStreamableHttpOptions { readonly routes?: readonly StreamableHttpRoute[]; readonly logger?: Logger; /** Reports listener failures after binding; request failures are logged instead. */ - readonly onerror?: (error: Error) => void; + readonly onerror?: (error: StreamableHttpListenError) => void; /** Close the server instance as the last stage of `close`; false when the factory's owner closes it. */ readonly closeInstance?: boolean; } @@ -557,7 +557,9 @@ export const serveStreamableHttp = async ( }); }); // Bind failures already rejected the listening wait above; only later listener errors are reported. - httpServer.on("error", (error) => options.onerror?.(error)); + httpServer.on("error", (error) => + options.onerror?.(new StreamableHttpListenError(error, "serving")), + ); let instance: RequestTrackingMcpServer; try { instance = await options.createMcpServer({ era: "legacy" }); diff --git a/src/server/http/streamableHttpListenError.ts b/src/server/http/streamableHttpListenError.ts index 7f536f9207..6a6982173f 100644 --- a/src/server/http/streamableHttpListenError.ts +++ b/src/server/http/streamableHttpListenError.ts @@ -1,10 +1,83 @@ -/** The listener could not bind before any client connected; the message names the system cause. */ +import { err, ok, type Result } from "../../domain/result.js"; + +/** Parsed system failure facts for listener recovery guidance. */ +export type StreamableHttpListenFailure = + | { + readonly kind: "host_lookup"; + readonly systemCode: + | "ENOTFOUND" + | "EAI_AGAIN" + | "EAI_FAIL" + | "EAI_NONAME" + | "ENODATA" + | "EAI_NODATA"; + } + | { readonly kind: "host_unavailable"; readonly systemCode: "EADDRNOTAVAIL" } + | { readonly kind: "port_in_use"; readonly systemCode: "EADDRINUSE" } + | { + readonly kind: "permission_denied"; + readonly systemCode: "EACCES" | "EPERM"; + }; + +export interface UnclassifiedStreamableHttpListenFailure { + readonly kind: "unclassified"; + readonly systemCode?: string; +} + +export type StreamableHttpListenClassification = Result< + StreamableHttpListenFailure, + UnclassifiedStreamableHttpListenFailure +>; + +const systemCode = (cause: unknown): string | undefined => { + if (typeof cause !== "object" || cause === null || !("code" in cause)) + return undefined; + const code: unknown = cause.code; + return typeof code === "string" ? code : undefined; +}; + +/** Normalize recognized Node listener system codes once, at the listener boundary. */ +const classify = (cause: unknown): StreamableHttpListenClassification => { + const code = systemCode(cause); + switch (code) { + case "ENOTFOUND": + case "EAI_AGAIN": + case "EAI_FAIL": + case "EAI_NONAME": + case "ENODATA": + case "EAI_NODATA": + return ok({ kind: "host_lookup", systemCode: code }); + case "EADDRNOTAVAIL": + return ok({ kind: "host_unavailable", systemCode: "EADDRNOTAVAIL" }); + case "EADDRINUSE": + return ok({ kind: "port_in_use", systemCode: "EADDRINUSE" }); + case "EACCES": + case "EPERM": + return ok({ kind: "permission_denied", systemCode: code }); + default: + return err({ + kind: "unclassified", + ...(code === undefined ? {} : { systemCode: code }), + }); + } +}; + +export type StreamableHttpListenPhase = "startup" | "serving"; + +/** Listener failure retaining its native cause, phase, and parsed recovery facts. */ export class StreamableHttpListenError extends Error { override readonly name = "StreamableHttpListenError"; - constructor(cause: unknown) { + readonly classification: StreamableHttpListenClassification; + readonly phase: StreamableHttpListenPhase; + + constructor(cause: unknown, phase: StreamableHttpListenPhase = "startup") { super( - `MCP HTTP listener could not bind: ${cause instanceof Error ? cause.message : String(cause)}`, + phase === "startup" + ? `MCP HTTP listener startup failed: ${cause instanceof Error ? cause.message : String(cause)}` + : `MCP HTTP listener failed after binding: ${cause instanceof Error ? cause.message : String(cause)}`, { cause }, ); + this.phase = phase; + this.classification = classify(cause); } } diff --git a/tests/boundary/cli/environmentServeRemediation.test.ts b/tests/boundary/cli/environmentServeRemediation.test.ts new file mode 100644 index 0000000000..449feec89d --- /dev/null +++ b/tests/boundary/cli/environmentServeRemediation.test.ts @@ -0,0 +1,179 @@ +import { createServer } from "node:http"; +import { once } from "node:events"; +import { randomUUID } from "node:crypto"; + +import { expect, it } from "vitest"; + +import { transportError } from "../../../src/cli/environmentCommands.js"; +import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js"; +import { StreamableHttpListenError } from "../../../src/server/http/streamableHttpListenError.js"; +import { + writeEnvironmentServeFiles, + type EnvironmentServeFiles, +} from "../../fixtures/environmentServe.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest, type TestCli } from "../../support/cli/cliFixture.js"; + +const files = async (): Promise => { + const directory = await createTestTempDirectory("rea-serve-remediation-"); + return writeEnvironmentServeFiles(directory); +}; + +const serve = async ( + cli: TestCli, + input: EnvironmentServeFiles, + host: string, + port: number, +) => { + const result = await cli.run({ + arguments: [ + "environment-serve", + input.manifestPath, + "--evidence", + input.capturePath, + "--host", + host, + "--port", + String(port), + "--json", + ], + timeoutMs: 30_000, + }); + return { + exitCode: result.exitCode, + output: JSON.parse(result.stdout) as Record, + }; +}; + +cliTest( + "reports host lookup failure with host-specific recovery and no listener startup", + async ({ cli }) => { + const input = await files(); + const host = `rea-${randomUUID()}.invalid`; + const result = await serve(cli, input, host, 0); + + expect(result.exitCode).toBe(1); + expect(result.output).toMatchObject({ + code: "resource_constraint", + details: { + operation: "environment-serve", + resource: "transport", + reported_limits: { host, port: 0 }, + diagnostics: { + listener_failure: { + kind: "host_lookup", + system_code: "ENOTFOUND", + }, + }, + }, + }); + expect(result.output.remediation).toMatchObject({ + action: expect.stringContaining("--host or MCP_HOST"), + }); + expect(JSON.stringify(result.output)).toContain("ENOTFOUND"); + expect(JSON.stringify(result.output)).not.toContain( + "Choose another --port", + ); + }, +); + +cliTest( + "retains port advice without closing the loopback listener it does not own", + async ({ cli }) => { + const input = await files(); + const owner = createServer(); + try { + owner.listen({ host: "127.0.0.1", port: 0 }); + await once(owner, "listening"); + const address = owner.address(); + if (address === null || typeof address === "string") + throw new Error("Owned loopback listener has no TCP address"); + + const result = await serve(cli, input, "127.0.0.1", address.port); + expect(result.exitCode).toBe(1); + expect(result.output).toMatchObject({ + code: "resource_constraint", + details: { + reported_limits: { host: "127.0.0.1", port: address.port }, + diagnostics: { + listener_failure: { + kind: "port_in_use", + system_code: "EADDRINUSE", + }, + }, + }, + remediation: { + action: expect.stringContaining("Choose another --port or MCP_PORT"), + }, + }); + expect(owner.listening).toBe(true); + } finally { + await new Promise((resolve, reject) => + owner.close((cause) => + cause === undefined ? resolve() : reject(cause), + ), + ); + } + }, +); + +it.each([ + ["EACCES", "permission_denied", "permitted to bind"], + ["EPERM", "permission_denied", "permitted to bind"], + ["EADDRNOTAVAIL", "host_unavailable", "address assigned to this machine"], + ["EIO", "unclassified", "Check the selected listener host, port"], +] as const)( + "projects typed listener failure %s without parsing the error display text", + (systemCode, kind, remediationText) => { + const cause = Object.assign( + new Error( + systemCode === "EIO" + ? "display mentions EADDRINUSE, but errno is EIO" + : "controlled listener failure", + ), + { code: systemCode }, + ); + const listenError = new StreamableHttpListenError(cause); + const transportFailure = transportError( + { host: "127.0.0.1", port: 80 }, + listenError, + ); + const projected = projectAnalysisError(transportFailure); + + expect(listenError.cause).toBe(cause); + expect(transportFailure.cause).toBe(listenError); + expect(projected.details).toMatchObject({ + diagnostics: { + listener_failure: { + kind, + system_code: systemCode, + }, + listener_phase: "startup", + }, + }); + expect(projected.remediation.action).toContain(remediationText); + expect(projected.remediation.action).not.toContain( + "stop the process using it", + ); + }, +); + +it("retains an after-bind listener failure as a serving-phase error", () => { + const cause = Object.assign(new Error("socket failed after bind"), { + code: "EIO", + }); + const listenError = new StreamableHttpListenError(cause, "serving"); + const projected = projectAnalysisError( + transportError({ host: "127.0.0.1", port: 80 }, listenError), + ); + + expect(listenError.cause).toBe(cause); + expect(listenError.message).toContain("failed after binding"); + expect(listenError.message).not.toContain("could not bind"); + expect(projected.details).toMatchObject({ + diagnostics: { + listener_failure: { kind: "unclassified", system_code: "EIO" }, + listener_phase: "serving", + }, + }); +});