fix(cli): report web readiness probe failures (#169)

Co-authored-by: Yaowei Zheng <hiyouga@buaa.edu.cn>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

packages/cli/src/commands/serve.ts

@@ -18,7 +18,16 @@ import { spawn } from "node:child_process";
 import path from "node:path";
 import { DEFAULT_SERVER_PORT } from "@prismshadow/penguin-core";
 import type { Command } from "commander";
-import type { Messages } from "../i18n.js";
+import type { Messages, WebProbeFailureKind } from "../i18n.js";
+
+/** Why the readiness poll gave up: failure class plus a one-line diagnostic from the last probe. */
+export interface ReadinessFailure {
+  kind: WebProbeFailureKind;
+  detail: string;
+}
+
+/** Result of `waitForReady`: ready, or timed out with the retained last probe failure. */
+export type ReadinessResult = { ready: true } | { ready: false; failure: ReadinessFailure };
 
 /** Default service port — core's DEFAULT_SERVER_PORT (7364), the single source of truth. */
 export const DEFAULT_PORT = DEFAULT_SERVER_PORT;
@@ -96,9 +105,103 @@ async function startServer(opts: {
   return { host, port };
 }
 
-/** Polls the service root path until it responds (any HTTP response counts as ready); keeps waiting on connection failure, returns false on timeout. */
-async function waitForReady(url: string, timeoutMs = 15_000, intervalMs = 300): Promise<boolean> {
+function errorProperty(
+  value: unknown,
+  property: "cause" | "code" | "errors" | "message" | "name",
+): unknown {
+  if ((typeof value !== "object" && typeof value !== "function") || value === null) {
+    return undefined;
+  }
+  return (value as Record<string, unknown>)[property];
+}
+
+/**
+ * Turns Node/undici's nested `fetch failed` errors into a stable one-line diagnostic.
+ * The useful code usually lives on `error.cause` rather than the top-level TypeError.
+ * Exported for unit tests.
+ */
+export function describeReadinessFailure(error: unknown): ReadinessFailure {
+  const chain: unknown[] = [];
+  const seen = new Set<unknown>();
+  let current: unknown = error;
+  while (current !== undefined && current !== null && !seen.has(current)) {
+    chain.push(current);
+    seen.add(current);
+    // Follow `cause` first; an AggregateError (e.g. localhost resolving to several
+    // addresses, all refused) keeps the real connect errors in `errors` instead.
+    const cause = errorProperty(current, "cause");
+    if (cause !== undefined && cause !== null) {
+      current = cause;
+      continue;
+    }
+    const errors = errorProperty(current, "errors");
+    current = Array.isArray(errors) ? errors[0] : undefined;
+  }
+
+  const code = chain
+    .map((item) => errorProperty(item, "code"))
+    .find((value): value is string => typeof value === "string" && value.length > 0);
+  const selected = [...chain].reverse().find((item) => {
+    const value = errorProperty(item, "message");
+    // Skip empty messages: an AggregateError's own message is "" and would mask real text.
+    return typeof value === "string" && value.length > 0;
+  });
+  const messageValue = selected === undefined ? undefined : errorProperty(selected, "message");
+  const nameValue = selected === undefined ? undefined : errorProperty(selected, "name");
+  const message =
+    typeof messageValue === "string" && messageValue.length > 0
+      ? messageValue
+      : typeof error === "string"
+        ? error
+        : "Unknown readiness probe error";
+  const name = typeof nameValue === "string" && nameValue !== "Error" ? nameValue : undefined;
+  const detail =
+    code !== undefined
+      ? `${code}: ${message}`
+      : name !== undefined
+        ? `${name}: ${message}`
+        : message;
+  const signature = chain
+    .flatMap((item) => [
+      errorProperty(item, "code"),
+      errorProperty(item, "name"),
+      errorProperty(item, "message"),
+    ])
+    .filter((value): value is string => typeof value === "string")
+    .join(" ")
+    .toUpperCase();
+
+  let kind: WebProbeFailureKind = "unknown";
+  if (signature.includes("ECONNREFUSED")) kind = "refused";
+  else if (
+    signature.includes("ETIMEDOUT") ||
+    signature.includes("TIMEOUT") ||
+    signature.includes("ABORTERROR")
+  ) {
+    kind = "timeout";
+  } else if (
+    signature.includes("ECONNRESET") ||
+    signature.includes("UND_ERR_SOCKET") ||
+    signature.includes("EPIPE")
+  ) {
+    kind = "reset";
+  } else if (signature.includes("EACCES") || signature.includes("EPERM")) {
+    kind = "permission";
+  } else if (signature.includes("ENOTFOUND") || signature.includes("EAI_AGAIN")) {
+    kind = "dns";
+  }
+
+  return { kind, detail };
+}
+
+/** Polls the service root path until it responds (any HTTP response counts as ready); retains the last connection failure for diagnostics on timeout. Exported for unit tests. */
+export async function waitForReady(
+  url: string,
+  timeoutMs = 15_000,
+  intervalMs = 300,
+): Promise<ReadinessResult> {
   const deadline = Date.now() + timeoutMs;
+  let lastError: unknown;
   for (;;) {
     try {
       // Each probe is capped at 1s: if the port is held by a non-HTTP program, the
@@ -109,11 +212,14 @@ async function waitForReady(url: string, timeoutMs = 15_000, intervalMs = 300):
       // so treat that 302 as ready rather than chasing it to a name that may resolve to ::1.
       const res = await fetch(url, { redirect: "manual", signal: AbortSignal.timeout(1000) });
       void res.body?.cancel();
-      return true;
-    } catch {
-      // The service isn't listening yet (or this probe timed out): keep polling.
+      return { ready: true };
+    } catch (error) {
+      // The service isn't listening yet (or this probe timed out): retain the reason and keep polling.
+      lastError = error;
+    }
+    if (Date.now() >= deadline) {
+      return { ready: false, failure: describeReadinessFailure(lastError) };
     }
-    if (Date.now() >= deadline) return false;
     await new Promise((resolve) => setTimeout(resolve, intervalMs));
   }
 }
@@ -149,9 +255,11 @@ export function registerServeCommands(program: Command, t: Messages): void {
     .action(async (opts: { port?: string; host?: string; open: boolean }) => {
       const { host, port } = await startServer(opts);
       const url = browserUrl(host, port);
-      const ready = await waitForReady(url);
-      if (!ready) {
-        process.stdout.write(`${t.webTimeout(url)}\n`);
+      const readiness = await waitForReady(url);
+      if (!readiness.ready) {
+        process.stderr.write(
+          `${t.webProbeFailed(url, readiness.failure.detail, readiness.failure.kind, port)}\n`,
+        );
         return;
       }
       process.stdout.write(`${t.webReady(url)}\n`);

packages/cli/src/i18n.ts

@@ -9,6 +9,10 @@
 /** UI language. */
 export type Language = "en" | "zh";
 
+/** Readiness probe failure classes; selects which hint `webProbeFailed` appends. */
+export type WebProbeFailureKind =
+  "timeout" | "refused" | "reset" | "permission" | "dns" | "unknown";
+
 /** Resolve the language from the env var; `zh` matches exactly, everything else falls back to English (see comment #2). */
 export function resolveLanguage(): Language {
   const v = (process.env.PENGUIN_LANG ?? "").trim().toLowerCase();
@@ -219,8 +223,8 @@ export interface Messages {
   vaultListEmpty(): string;
   /** URL prompt once the `penguin web` service is ready. */
   webReady(url: string): string;
-  /** Manual-open prompt after the `penguin web` ready-poll times out (15s). */
-  webTimeout(url: string): string;
+  /** Diagnostic shown after the `penguin web` ready-poll times out (15s). */
+  webProbeFailed(url: string, detail: string, kind: WebProbeFailureKind, port: number): string;
 }
 
 function headerEn(
@@ -451,7 +455,22 @@ const en: Messages = {
   vaultListTitle: () => "Vault environment variables (values masked):",
   vaultListEmpty: () => "The vault is empty. Add one with `penguin config vault set`.",
   webReady: (url) => `Web UI ready: ${url}`,
-  webTimeout: (url) => `Server is not responding yet; open ${url} manually once it is ready.`,
+  webProbeFailed: (url, detail, kind, port) => {
+    const hint = {
+      timeout:
+        `The connection timed out. Check whether a firewall or security application is blocking it. ` +
+        `Allow PenguinHarness to communicate on local port ${port}.`,
+      refused:
+        "Nothing accepted the connection. Check whether the server exited or HOST/PORT points somewhere else.",
+      reset:
+        "The connection closed before an HTTP response. Check local security software and retry.",
+      permission:
+        "The operating system denied the connection. Check firewall or security policy permissions.",
+      dns: "The host name could not be resolved. Check --host or HOST.",
+      unknown: `Open ${url} manually after the server is ready.`,
+    }[kind];
+    return `Server readiness check failed for ${url}.\nLast probe error: ${detail}\n${hint}`;
+  },
 };
 
 const zh: Messages = {
@@ -645,7 +664,17 @@ const zh: Messages = {
   vaultListTitle: () => "vault 环境变量（值已掩码）：",
   vaultListEmpty: () => "vault 为空。用 `penguin config vault set` 添加。",
   webReady: (url) => `Web 界面已就绪：${url}`,
-  webTimeout: (url) => `服务尚未就绪，请稍后手动打开 ${url}。`,
+  webProbeFailed: (url, detail, kind, port) => {
+    const hint = {
+      timeout: `连接超时。请检查防火墙或安全软件是否拦截。请允许 PenguinHarness 在本机端口 ${port} 上通信。`,
+      refused: "没有进程接受连接。请检查服务是否已经退出，或 HOST/PORT 是否指向了其他地址。",
+      reset: "连接在收到 HTTP 响应前已关闭。请检查本机安全软件后重试。",
+      permission: "操作系统拒绝了连接。请检查防火墙或安全策略权限。",
+      dns: "无法解析主机名。请检查 --host 或 HOST。",
+      unknown: `请在服务就绪后手动打开 ${url}。`,
+    }[kind];
+    return `服务探活失败：${url}\n最后一次探测错误：${detail}\n${hint}`;
+  },
 };
 
 /** Get the message set for a language. */

packages/cli/test/serve.test.ts

@@ -1,4 +1,4 @@
-import { describe, expect, it } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
 import path from "node:path";
 import { Command } from "commander";
 import { DEFAULT_SERVER_PORT } from "@prismshadow/penguin-core";
@@ -8,8 +8,10 @@ import {
   browserCommand,
   browserUrl,
   cliEntryFor,
+  describeReadinessFailure,
   registerServeCommands,
   resolvePort,
+  waitForReady,
 } from "../src/commands/serve.js";
 import { getMessages } from "../src/i18n.js";
 
@@ -93,3 +95,93 @@ describe("cliEntryFor (the entry advertised for the web self-update)", () => {
     expect(cliEntryFor("")).toBeNull();
   });
 });
+
+describe("readiness probe diagnostics", () => {
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  it("reports a successful HTTP response as ready regardless of its status", async () => {
+    vi.spyOn(globalThis, "fetch").mockResolvedValue(new Response(null, { status: 503 }));
+
+    await expect(waitForReady("http://127.0.0.1:7364/", 0, 0)).resolves.toEqual({
+      ready: true,
+    });
+  });
+
+  it("keeps polling after failed probes and reports ready once a response arrives", async () => {
+    vi.spyOn(globalThis, "fetch")
+      .mockRejectedValueOnce(new TypeError("fetch failed"))
+      .mockResolvedValue(new Response(null, { status: 200 }));
+
+    await expect(waitForReady("http://127.0.0.1:7364/", 5_000, 0)).resolves.toEqual({
+      ready: true,
+    });
+  });
+
+  it("retains the nested undici error from the last failed probe", async () => {
+    const cause = Object.assign(new Error("Connect Timeout Error"), {
+      code: "UND_ERR_CONNECT_TIMEOUT",
+    });
+    vi.spyOn(globalThis, "fetch").mockRejectedValue(
+      Object.assign(new TypeError("fetch failed"), { cause }),
+    );
+
+    await expect(waitForReady("http://127.0.0.1:7364/", 0, 0)).resolves.toEqual({
+      ready: false,
+      failure: {
+        kind: "timeout",
+        detail: "UND_ERR_CONNECT_TIMEOUT: Connect Timeout Error",
+      },
+    });
+  });
+
+  it.each([
+    ["ECONNREFUSED", "refused"],
+    ["ECONNRESET", "reset"],
+    ["EACCES", "permission"],
+    ["ENOTFOUND", "dns"],
+  ] as const)("classifies %s failures as %s", (code, kind) => {
+    expect(describeReadinessFailure(Object.assign(new Error("probe failed"), { code }))).toEqual({
+      kind,
+      detail: `${code}: probe failed`,
+    });
+  });
+
+  it("classifies the probe's own 1s abort (DOMException TimeoutError) as a timeout", () => {
+    expect(
+      describeReadinessFailure(
+        new DOMException("The operation was aborted due to timeout", "TimeoutError"),
+      ),
+    ).toEqual({
+      kind: "timeout",
+      detail: "TimeoutError: The operation was aborted due to timeout",
+    });
+  });
+
+  it("digs the connect error out of an empty-message AggregateError (multi-address host)", () => {
+    const aggregate = Object.assign(
+      new AggregateError([
+        Object.assign(new Error("connect ECONNREFUSED ::1:7364"), { code: "ECONNREFUSED" }),
+        Object.assign(new Error("connect ECONNREFUSED 127.0.0.1:7364"), { code: "ECONNREFUSED" }),
+      ]),
+      { code: "ECONNREFUSED" },
+    );
+    const error = Object.assign(new TypeError("fetch failed"), { cause: aggregate });
+
+    expect(describeReadinessFailure(error)).toEqual({
+      kind: "refused",
+      detail: "ECONNREFUSED: connect ECONNREFUSED ::1:7364",
+    });
+  });
+
+  it("includes actionable localized firewall guidance for connection timeouts", () => {
+    const detail = "UND_ERR_CONNECT_TIMEOUT: Connect Timeout Error";
+    expect(
+      getMessages("en").webProbeFailed("http://127.0.0.1:7364/", detail, "timeout", 7364),
+    ).toContain("Allow PenguinHarness to communicate on local port 7364");
+    expect(
+      getMessages("zh").webProbeFailed("http://127.0.0.1:7364/", detail, "timeout", 7364),
+    ).toContain("请允许 PenguinHarness 在本机端口 7364 上通信");
+  });
+});