fix(runtime): stop pathological non-Error throws from wedging the error boundary (#180)

* fix(runtime): stop pathological non-Error throws from wedging the error boundary A thrown value with a throwing coercion/getter could make three sibling throw sites in the error-exit/display path re-throw with NO surrounding try/catch, wedging Vue's post-flush scheduler — the app hangs and waitUntilExit() never settles: - messageForNonError's two String(value) fallbacks (a throwing Symbol.toPrimitive/toString/valueOf) — now routed through a throw-safe safeString() returning "[unserializable value]". - isErrorInput's Object.prototype.toString.call (a throwing Symbol.toStringTag getter), which runs BEFORE messageForNonError on the error-exit path — now guarded; on throw the value is treated as non-Error and routed through messageForNonError. - ErrorOverview's `.stack` read (a throwing `.stack` getter) during render — now read exactly once under try/catch; on throw it renders header-only. Tests: unit coverage of messageForNonError plus an end-to-end "does not wedge" mount test for all three pathological shapes, and an overview-frame test proving the .stack guard is load-bearing for correctness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(runtime): close two more pathological-throw paths in the error boundary (Codex review) Final review of the wedge fix found two reachable throw sites it hadn't closed: - isErrorInput: `value instanceof Error` ran OUTSIDE the try/catch, but `instanceof` invokes the value's [[GetPrototypeOf]], which a Proxy with a throwing getPrototypeOf trap re-throws — wedging the boundary exactly like the Symbol.toStringTag case. Wrap the whole body (instanceof + brand check) in one try/catch → false on throw. (The old "instanceof CANNOT throw" comment was wrong.) - ErrorOverview source excerpt: a crafted/stale `.stack` can parse to an existing DIRECTORY, so fs.existsSync passes and fs.readFileSync throws EISDIR during render — repainting the overview for the EISDIR error while waitUntilExit() rejects the original (a displayed-vs-rejected e17 disagreement). Guard the file read; on failure render header-only (no excerpt). Tests: a Proxy whose getPrototypeOf throws does not wedge; a directory-pointing `.stack` renders header-only with display==reject; and an e2e assertion that "[unserializable value]" is both displayed AND rejected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-15 01:14:23 +08:00
parent be045cfaff
commit 670cca402a
6 changed files with 343 additions and 13 deletions
@@ -1,3 +1,4 @@
+import { cwd } from "node:process";
 import { defineComponent, h } from "vue";
 import { expect, test } from "vite-plus/test";
 import stripAnsi from "strip-ansi";
@@ -181,6 +182,40 @@ test("non-Error throw: overview message and rejected Error message are identical
  expect(reject.message).toBe(overviewMessage(frame));
 });

+test("non-Error throw: '[unserializable value]' is shown AND rejected with, and they agree", async () => {
+  // A pathological thrown value whose `.message` is a non-string AND whose
+  // primitive coercion throws: messageForNonError selects the String(value)
+  // branch, String(value) throws (Symbol.toPrimitive), and safeString() falls
+  // back to the fixed "[unserializable value]" placeholder. The SAME helper feeds
+  // the overview header and render.ts's reject-wrap, so the displayed and rejected
+  // messages must BOTH be "[unserializable value]" — they cannot drift (e17).
+  const Thrower = defineComponent(() => {
+    return () => {
+      // eslint-disable-next-line @typescript-eslint/only-throw-error -- non-Error value whose primitive coercion throws, exercising the unserializable placeholder (e17)
+      throw {
+        get message(): number {
+          return 42;
+        },
+        [Symbol.toPrimitive](): never {
+          throw new Error("toPrimitive boom");
+        },
+      };
+    };
+  });
+
+  const { frame, reject } = await renderFrameAndReject(Thrower);
+
+  // Display: the overview surfaces the placeholder.
+  expect(frame).toContain(" ERROR  [unserializable value]");
+  // Reject: the SAME placeholder string on a real Error.
+  expect(reject.kind).toBe("rejected");
+  if (reject.kind !== "rejected") throw new Error("expected rejection");
+  expect(reject.isError).toBe(true);
+  expect(reject.message).toBe("[unserializable value]");
+  // Consistency: display === reject.
+  expect(reject.message).toBe(overviewMessage(frame));
+});
+
 test("non-Error throw: non-string .message falls back to String on BOTH paths and they agree", async () => {
  const Thrower = defineComponent(() => {
    return () => {
@@ -258,6 +293,77 @@ test("unparsable stack frame falls back to literal backslash-t (not a real TAB)"
  expect(fallbackLine).not.toContain("\t");
 });

+test("throwing .stack getter renders ERROR header with no synthetic stack (hardened read)", async () => {
+  // A pathological thrown value with a THROWING `.stack` getter. ErrorOverview
+  // reads `.stack` during render; an unguarded read would throw, Vue would catch
+  // it and re-route a NEW Error (with a real `.stack` pointing into Vue/dist
+  // internals), and the boundary would re-render an overview that LEAKS those
+  // internal frames — the same synthetic-stack corruption the primitive test
+  // guards against. With the guarded single `.stack` read, the throw is swallowed
+  // and the overview renders header-only (the primitive-throw path).
+  const StackGetterThrower = defineComponent(() => {
+    return () => {
+      // eslint-disable-next-line @typescript-eslint/only-throw-error -- non-Error value with a throwing .stack getter, exercising the hardened read
+      throw {
+        message: "stack getter boom",
+        get stack(): string {
+          throw new Error("inner stack boom");
+        },
+      };
+    };
+  });
+
+  const frame = await renderErrorFrame(StackGetterThrower);
+
+  // The header shows the string .message (read via messageForNonError).
+  expect(frame).toContain(" ERROR  stack getter boom");
+
+  // No synthetic stack leaked: the unguarded-read regression surfaces as
+  // dist/render-to-string/runtime-core frames and "- " stack lines. None appear.
+  expect(frame).not.toContain("render-to-string");
+  expect(frame).not.toContain("runtime-core");
+  expect(frame).not.toContain("dist/");
+  expect(frame).not.toMatch(/^\s*- /m);
+});
+
+test("stack origin pointing at a DIRECTORY renders header-only without leaking EISDIR", async () => {
+  // A crafted/stale `.stack` can parse to an existing DIRECTORY path. Before the
+  // fix, ErrorOverview's excerpt block did `fs.existsSync(dir)` (true for a dir) →
+  // `fs.readFileSync(dir)` throws EISDIR DURING render. The boundary then re-faults
+  // and repaints an overview for the EISDIR error while waitUntilExit() rejects with
+  // the ORIGINAL message — a displayed-vs-rejected DISAGREEMENT (violates e17). With
+  // the excerpt read guarded, the read failure is swallowed (no excerpt) and the
+  // overview renders the header/origin for the original error.
+  //
+  // We point the first frame at process.cwd() (a real directory on disk) so
+  // fs.existsSync passes but fs.readFileSync throws EISDIR.
+  const dirPath = cwd();
+  const DirStackThrower = defineComponent(() => {
+    return () => {
+      const e = new Error("Dir stack boom");
+      const firstLine = (e.stack ?? "").split("\n")[0] ?? "Error: Dir stack boom";
+      // A parseable frame whose file is an existing directory.
+      e.stack = `${firstLine}\n    at someFn (${dirPath}:1:1)`;
+      throw e;
+    };
+  });
+
+  const { frame, reject } = await renderFrameAndReject(DirStackThrower);
+
+  // Display: header shows the ORIGINAL message, not an EISDIR error.
+  expect(frame).toContain(" ERROR  Dir stack boom");
+  expect(frame).not.toContain("EISDIR");
+  expect(frame).not.toContain("illegal operation on a directory");
+
+  // Reject: waitUntilExit() rejects with the ORIGINAL Error.
+  expect(reject.kind).toBe("rejected");
+  if (reject.kind !== "rejected") throw new Error("expected rejection");
+  expect(reject.isError).toBe(true);
+  expect(reject.message).toBe("Dir stack boom");
+  // Consistency: displayed message === rejected message (no EISDIR leakage).
+  expect(reject.message).toBe(overviewMessage(frame));
+});
+
 test("primitive (non-Error) throw renders ERROR header with no synthetic stack", async () => {
  const frame = await renderErrorFrame(PrimitiveThrower);

@@ -0,0 +1,67 @@
+import { expect, test } from "vite-plus/test";
+import { messageForNonError } from "@vue-tui/runtime/internal";
+
+// messageForNonError feeds the error-display / reject path: render.ts's
+// onErrorCaptured wraps `new Error(messageForNonError(err))` with NO surrounding
+// try/catch (render.ts ~523, and the errorHandler exit path ~1276). Its docstring
+// promises it "must not itself throw on a pathological thrown object", so every
+// coercion inside it has to be throw-safe.
+//
+// Imported from the built `@vue-tui/runtime/internal` dist (not source): the
+// source module imports box.vue/text.vue, and the runtime-tests vitest config
+// has no @vitejs/plugin-vue, so a source-relative import fails to compile.
+
+test("normal values coerce to their expected message", () => {
+  expect(messageForNonError(42)).toBe("42");
+  expect(messageForNonError("boom")).toBe("boom");
+  expect(messageForNonError({ message: "hi" })).toBe("hi");
+  expect(messageForNonError(null)).toBe("null");
+  expect(messageForNonError(undefined)).toBe("undefined");
+  expect(messageForNonError(new Error("x"))).toBe("x");
+});
+
+test("a throwing .message getter falls back without throwing", () => {
+  // The `.message` READ throws; String(value) on the plain object then yields
+  // "[object Object]" safely (toString is the default). Exercises the catch
+  // branch's coercion on a value that DOES coerce.
+  const pathological = {
+    get message(): string {
+      throw new Error("message getter boom");
+    },
+  };
+  expect(() => messageForNonError(pathological)).not.toThrow();
+  expect(messageForNonError(pathological)).toBe("[object Object]");
+});
+
+test("a value whose primitive coercion throws does not throw (the confirmed bug)", () => {
+  // HIGH-severity bug: `.message` is a non-string (so the typeof check selects
+  // the String(value) branch), and String(value) itself throws because
+  // Symbol.toPrimitive throws. Before the fix, messageForNonError re-throws here,
+  // wedging render.ts's onErrorCaptured hook (the app hangs, waitUntilExit()
+  // never settles).
+  const pathological = {
+    get message(): number {
+      return 42;
+    },
+    [Symbol.toPrimitive](): never {
+      throw new Error("toPrimitive boom");
+    },
+  };
+  expect(() => messageForNonError(pathological)).not.toThrow();
+  expect(messageForNonError(pathological)).toBe("[unserializable value]");
+});
+
+test("a value that throws in BOTH the message read and String() does not throw", () => {
+  // Both guarded spots fire: the message getter throws (catch branch), and the
+  // catch branch's String(value) also throws because toString throws.
+  const pathological = {
+    get message(): string {
+      throw new Error("message getter boom");
+    },
+    toString(): never {
+      throw new Error("toString boom");
+    },
+  };
+  expect(() => messageForNonError(pathological)).not.toThrow();
+  expect(messageForNonError(pathological)).toBe("[unserializable value]");
+});
@@ -0,0 +1,103 @@
+import { defineComponent } from "vue";
+import { expect, test } from "vite-plus/test";
+import { createApp } from "@vue-tui/runtime";
+import { makeFakeWritable, makeFakeStdin } from "./test-streams.ts";
+
+// End-to-end proof that a PATHOLOGICAL non-Error throw never WEDGES the error
+// boundary. The original bug: a thrown value with a throwing coercion/getter
+// makes one of the three sibling throw sites in the error-exit/display path
+// re-throw with NO surrounding try/catch, which wedges Vue's post-flush
+// scheduler — the app hangs and waitUntilExit() never settles. We mount a real
+// app whose user component throws each shape and assert waitUntilExit() SETTLES
+// (rejects) rather than hanging.
+//
+// Non-sequential: these mount/unmount apps but assert ONLY on the app's own
+// waitUntilExit() outcome, never on process-global state (yoga counts,
+// listenerCount, fake timers), so file-level parallelism can't perturb them.
+//
+// A wedge means the promise NEVER settles, which would hang the test until
+// vitest's timeout. To keep RED fast and the failure legible, race
+// waitUntilExit() against a short timeout and assert the settle won.
+const WEDGE_TIMEOUT = Symbol("timeout");
+
+async function expectMountDoesNotWedge(thrown: unknown): Promise<void> {
+  const App = defineComponent(() => () => {
+    throw thrown;
+  });
+
+  const stdout = makeFakeWritable();
+  const stderr = makeFakeWritable();
+  const { stream: stdin } = makeFakeStdin();
+
+  const app = createApp(App);
+  app.mount({ stdout, stdin, stderr, debug: true, exitOnCtrlC: false });
+
+  const outcome = await Promise.race([
+    app
+      .waitUntilExit()
+      .then(() => "resolved" as const)
+      .catch(() => "rejected" as const),
+    new Promise<typeof WEDGE_TIMEOUT>((r) => setTimeout(() => r(WEDGE_TIMEOUT), 1500)),
+  ]);
+
+  // Not wedged: the promise settled before the timeout fired.
+  expect(outcome).not.toBe(WEDGE_TIMEOUT);
+  // A throwing component routes through exit(err) → REJECTS waitUntilExit().
+  expect(outcome).toBe("rejected");
+
+  app.unmount();
+}
+
+test("a throwing Symbol.toPrimitive (messageForNonError path) does not wedge the boundary", async () => {
+  // `.message` is a non-string, so messageForNonError selects the String(value)
+  // branch; String(value) then invokes the throwing Symbol.toPrimitive. Guarded
+  // by safeString() inside messageForNonError.
+  await expectMountDoesNotWedge({
+    get message(): number {
+      return 42;
+    },
+    [Symbol.toPrimitive](): never {
+      throw new Error("toPrimitive boom");
+    },
+  });
+});
+
+test("a throwing Symbol.toStringTag getter (isErrorInput path) does not wedge the boundary", async () => {
+  // isErrorInput() does `Object.prototype.toString.call(value)`, which READS
+  // `value[Symbol.toStringTag]`. A throwing getter there makes isErrorInput
+  // re-throw on the error-exit path. Guarded by the try/catch in isErrorInput.
+  await expectMountDoesNotWedge({
+    get [Symbol.toStringTag](): string {
+      throw new Error("toStringTag boom");
+    },
+  });
+});
+
+test("a throwing .stack getter (ErrorOverview path) does not wedge the boundary", async () => {
+  // ErrorOverview reads `.stack` off the raw thrown value during render. A
+  // throwing getter there makes the overview render throw. Guarded by the
+  // try/catch around the single `.stack` read in ErrorOverview.
+  await expectMountDoesNotWedge({
+    get stack(): string {
+      throw new Error("stack boom");
+    },
+  });
+});
+
+test("a Proxy with a throwing getPrototypeOf trap (isErrorInput path) does not wedge the boundary", async () => {
+  // isErrorInput() does `value instanceof Error`, which invokes the value's
+  // [[GetPrototypeOf]]. A Proxy with a throwing getPrototypeOf trap makes that
+  // `instanceof` re-throw on the error-exit path — a GENUINE wedge, since
+  // isErrorInput runs in onErrorCaptured / resolveExit / appContext.exit with no
+  // outer guard. Guarded by wrapping the whole isErrorInput body in try/catch.
+  await expectMountDoesNotWedge(
+    new Proxy(
+      {},
+      {
+        getPrototypeOf(): never {
+          throw new Error("proto boom");
+        },
+      },
+    ),
+  );
+});