diff --git a/.agents/docs/ink-divergences.md b/.agents/docs/ink-divergences.md index e87de1b..918c111 100644 --- a/.agents/docs/ink-divergences.md +++ b/.agents/docs/ink-divergences.md @@ -333,21 +333,27 @@ These divergences are deliberate, but they are not strict supersets and are not driven by Vue's framework model or API conventions. vue-tui intentionally chooses a different runtime behavior, ownership rule, or out-of-contract handling. -### Non-`Error` thrown values keep their message in the error overview +### Non-`Error` thrown values: uniform show-the-error-and-reject -- **Ink:** Ink **accepts** the throw — a thrown non-`Error` (`throw 'boom'`) is caught by its - ErrorBoundary and rendered through `ErrorOverview` (verified by running real Ink: an - `ERROR` overview with a **blank** message, exit 0, no crash) because `error.message` is - `undefined` for a string throw. -- **vue-tui:** the error boundary keeps the **raw** thrown value and `ErrorOverview` shows - `String(value)` as the message, so `throw 'boom'` renders ` ERROR boom`, not a blank - `ERROR`. Like Ink, no stack block is rendered when the value carries no stack. -- **Why:** same input, different output — **not** an additive superset (both accept the - non-`Error` throw; vue-tui takes no extra input), so this is an intentional choice. Showing - `String(value)` gives a useful message for the (lint-discouraged) non-`Error` throw and - keeps the message vue-tui surfaced before it stopped wrapping such throws in - `new Error(String(value))` (which produced a misleading synthetic stack pointing at - framework internals; that stack is now gone). Introduced 2026-05-31. +- **Ink:** accepts the throw, but its handling is **non-uniform** (run-verified vs v7.0.4): + for a **truthy** non-`Error` it renders an `ErrorOverview` showing a string `.message` + (**blank** for a bare string, since `'boom'.message` is `undefined`) **and RESOLVES** + `waitUntilExit()` with the **raw** thrown value — a throw looks like a clean exit. For a + **falsy** throw (`0` / `''` / `null`) it renders **no** overview and leaves + `waitUntilExit()` **PENDING** (recoverable — a later unmount resolves it with `undefined`). +- **vue-tui:** **any** thrown value renders an `ErrorOverview` (message = a string `.message` + if present, else `String(value)`) **and REJECTS** `waitUntilExit()` with an `Error` whose + `.message` **EQUALS the displayed message** — one `messageForNonError(value)` helper feeds + both the overview header and the reject-wrap, so display and reject can't drift (e17). No + synthetic stack (a value with no `.stack` renders only the header). +- **Why:** aligning to Ink reduces bugs only where Ink is correct. Ink resolving the exit + promise with a thrown value, and silently hanging on a falsy throw, are abnormal, so + vue-tui deliberately diverges to one uniform contract: show the error, reject the exit. Same + recover-vs-crash family as the invalid-input-validation divergence. Showing a real message + (string `.message` else `String(value)`) is useful for the lint-discouraged non-`Error` + throw, and matching the rejected message to it removes a confusing internal inconsistency + (`throw {message:'x'}` once displayed `x` but rejected `[object Object]`). Introduced + 2026-05-31; consistency fixed 2026-06-12. Maintainer decision (2026-06-12): KEEP. ### Second `mount()` on a live stdout is an inert no-op diff --git a/packages/runtime-tests/integration/lifecycle/error-overview.test.tsx b/packages/runtime-tests/integration/lifecycle/error-overview.test.tsx index 6cd67b7..1663ac3 100644 --- a/packages/runtime-tests/integration/lifecycle/error-overview.test.tsx +++ b/packages/runtime-tests/integration/lifecycle/error-overview.test.tsx @@ -86,6 +86,120 @@ async function renderErrorFrame(component: Parameters[0]): Pro return stripAnsi(lastContentWrite); } +// Capture BOTH the painted ERROR overview frame AND what waitUntilExit() rejects +// with, from a SINGLE mount of the same throwing component. This is what proves +// display/reject CONSISTENCY: the message shown to the user and the message on +// the rejected Error must be the same string (audit finding e17). +async function renderFrameAndReject(component: Parameters[0]): Promise<{ + frame: string; + reject: { kind: "rejected"; message: unknown; isError: boolean } | { kind: "resolved" }; +}> { + const stdout = makeFakeWritable(); + const stderr = makeFakeWritable(); + const { stream: stdin } = makeFakeStdin(); + const writes = captureWrites(stdout); + + const app = createApp(component); + app.mount({ stdout, stdin, stderr, debug: true, exitOnCtrlC: false }); + + let reject: { kind: "rejected"; message: unknown; isError: boolean } | { kind: "resolved" } = { + kind: "resolved", + }; + const settled = app.waitUntilExit().then( + () => { + reject = { kind: "resolved" }; + }, + (e: unknown) => { + reject = { kind: "rejected", message: (e as Error)?.message, isError: e instanceof Error }; + }, + ); + + await new Promise((r) => setImmediate(r)); + await new Promise((r) => setImmediate(r)); + await settled; + + const content = getContentWrites(writes); + const lastContentWrite = content.at(-1); + if (lastContentWrite === undefined) { + throw new Error("no content write captured"); + } + return { frame: stripAnsi(lastContentWrite), reject }; +} + +// Pull the message that follows the white-on-red " ERROR " label out of the +// painted frame (the text the user actually sees as the error message). +function overviewMessage(frame: string): string { + const header = frame.split("\n").find((l) => l.includes("ERROR")); + if (header === undefined) throw new Error("no ERROR header line in frame"); + // Label renders as " ERROR " (space-padded) then " " (a leading + // space before the message). Strip the label + all surrounding whitespace to + // recover the pure message text. + return header.replace(/^\s*ERROR\s*/, "").trimEnd(); +} + +// --- Display/reject consistency (audit finding e17) --- +// vue-tui's blessed contract: ANY thrown value renders an ErrorOverview AND +// rejects waitUntilExit() with an Error whose .message EQUALS the displayed +// message. Before the fix, `throw {message:'objmsg'}` DISPLAYED "objmsg" but +// REJECTED with "[object Object]" (the wrap site used new Error(String(err))). + +test("non-Error throw: overview message and rejected Error message are identical (object with string message)", async () => { + const Thrower = defineComponent(() => { + return () => { + // eslint-disable-next-line @typescript-eslint/only-throw-error -- exercising a non-Error throw with a string .message (e17) + throw { message: "objmsg" }; + }; + }); + + const { frame, reject } = await renderFrameAndReject(Thrower); + + // Display: the overview surfaces the string .message. + expect(frame).toContain(" ERROR objmsg"); + // Reject: the SAME message, not "[object Object]". + expect(reject.kind).toBe("rejected"); + if (reject.kind !== "rejected") throw new Error("expected rejection"); + expect(reject.isError).toBe(true); + expect(reject.message).toBe("objmsg"); + // Consistency: display === reject. + expect(reject.message).toBe(overviewMessage(frame)); +}); + +test("non-Error throw: overview message and rejected Error message are identical (number)", async () => { + const Thrower = defineComponent(() => { + return () => { + // eslint-disable-next-line @typescript-eslint/only-throw-error -- exercising a primitive non-Error throw (e17) + throw 42; + }; + }); + + const { frame, reject } = await renderFrameAndReject(Thrower); + + expect(frame).toContain(" ERROR 42"); + expect(reject.kind).toBe("rejected"); + if (reject.kind !== "rejected") throw new Error("expected rejection"); + expect(reject.message).toBe("42"); + 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 () => { + // A NON-string .message: the overview's `typeof message === 'string'` guard + // fails, so both paths fall back to String(value). They must still AGREE. + // eslint-disable-next-line @typescript-eslint/only-throw-error -- exercising a non-Error throw with a non-string .message (e17) + throw { message: 42 }; + }; + }); + + const { frame, reject } = await renderFrameAndReject(Thrower); + + expect(reject.kind).toBe("rejected"); + if (reject.kind !== "rejected") throw new Error("expected rejection"); + // Both fall back to String({message:42}) === "[object Object]". + expect(reject.message).toBe("[object Object]"); + expect(reject.message).toBe(overviewMessage(frame)); +}); + test("renders a full ERROR overview frame with label, origin, excerpt, and stack", async () => { const frame = await renderErrorFrame(ThrowingComponent); @@ -149,8 +263,8 @@ test("primitive (non-Error) throw renders ERROR header with no synthetic stack", // vue-tui renders String(value) as the message for a primitive throw, so the header // shows the thrown text. (Ink renders {error.message}, blank for a primitive that has no - // .message — see .agents/docs/ink-divergences.md, section "Non-Error thrown values keep - // their message in the error overview".) + // .message — see .agents/docs/ink-divergences.md, section "Non-Error thrown values: + // uniform show-the-error-and-reject".) expect(frame).toContain(" ERROR primitive thrown"); // A primitive has no .stack, so Ink renders no origin/excerpt/stack block. diff --git a/packages/runtime/src/components/error-overview.ts b/packages/runtime/src/components/error-overview.ts index 9b66f40..637d88d 100644 --- a/packages/runtime/src/components/error-overview.ts +++ b/packages/runtime/src/components/error-overview.ts @@ -22,6 +22,28 @@ const stackUtils = new StackUtils({ internals: StackUtils.nodeInternals(), }); +// The user-facing message for a NON-Error thrown value. SINGLE source of truth +// shared by the ErrorOverview header (what the user SEES) and render.ts's +// reject-wrap (`new Error(messageForNonError(value))`, what waitUntilExit() +// REJECTS with), so the displayed and rejected messages can never drift apart +// (audit finding e17). Prefer a string `.message` (covers `throw {message:'x'}` +// and a cross-realm Error read structurally); otherwise fall back to +// `String(value)` (covers `throw 'boom'`, `throw 42`, a non-string `.message`). +export function messageForNonError(value: unknown): string { + // Read `.message` exactly once: the typecheck and the returned value must see + // the SAME read, and the read is guarded because this feeds the error-display + // / reject path — it must not itself throw on a pathological thrown object + // (e.g. a `.message` getter that throws), which the old `String(value)` form + // never touched. Fall back to `String(value)` on any failure, as before. + let message: unknown; + try { + message = (value as { message?: unknown })?.message; + } catch { + return String(value); + } + return typeof message === "string" ? message : String(value); +} + export const ErrorOverview = defineComponent({ name: "ErrorOverview", props: { @@ -43,11 +65,10 @@ export const ErrorOverview = defineComponent({ : undefined; // Ink renders `{error.message}`. A cross-realm Error has a different // prototype and fails `instanceof Error`, so read a string `.message` - // structurally; primitives still fall back to String(value). - const errorMessage = - typeof (error as { message?: unknown })?.message === "string" - ? (error as { message: string }).message - : String(error); + // structurally; primitives still fall back to String(value). The same + // helper computes the message render.ts rejects waitUntilExit() with, so + // the shown and rejected messages stay identical (e17). + const errorMessage = messageForNonError(error); // First stack line is the message; the rest are frames. The first frame // is the throw origin used for the file:line:col header and excerpt. diff --git a/packages/runtime/src/render.ts b/packages/runtime/src/render.ts index eb9cc1b..8dd6eac 100644 --- a/packages/runtime/src/render.ts +++ b/packages/runtime/src/render.ts @@ -44,7 +44,7 @@ import { } from "./context.ts"; import { devState, DevStateKey, initHmrBridge } from "./hmr.ts"; import { createDevOverlayWrapper } from "./overlay.ts"; -import { ErrorOverview } from "./components/error-overview.ts"; +import { ErrorOverview, messageForNonError } from "./components/error-overview.ts"; import { resolveSize } from "./composables/useTerminalSize.ts"; export interface MountOptions { @@ -502,8 +502,10 @@ export function createApp(root: Component, rootProps?: RootProps | null): TuiApp // `instanceof Error`, passes the `[object Error]` brand check) — so the // ORIGINAL thrown error reaches exit()/waitUntilExit() unchanged, // matching Ink's ErrorBoundary (rejects with the thrown value itself). - // A true non-Error throw (`throw "x"`, `throw 0`) is still wrapped. - const e = isErrorInput(err) ? err : new Error(String(err)); + // A true non-Error throw (`throw "x"`, `throw 0`, `throw {message:'x'}`) + // is wrapped with the SAME message ErrorOverview displays + // (messageForNonError), so the shown and rejected messages agree (e17). + const e = isErrorInput(err) ? err : new Error(messageForNonError(err)); caught.value = err; errored.value = true; // Flush the ErrorOverview frame, then exit @@ -1218,8 +1220,9 @@ export function createApp(root: Component, rootProps?: RootProps | null): TuiApp // false to stop propagation, so caught errors won't reach here. baseApp.config.errorHandler = (err) => { // Preserve a genuine (incl. cross-realm) Error so the original survives to - // exit(); only wrap a true non-Error. See isErrorInput / onErrorCaptured. - appContext.exit(isErrorInput(err) ? err : new Error(String(err))); + // exit(); only wrap a true non-Error — with the SAME message ErrorOverview + // displays (messageForNonError, e17). See isErrorInput / onErrorCaptured. + appContext.exit(isErrorInput(err) ? err : new Error(messageForNonError(err))); }; // Only listen for resize in interactive mode (matching Ink).