From aa1c8bfb0785a4aad1167013e7567e5678e07d71 Mon Sep 17 00:00:00 2001 From: Yunfei He Date: Sun, 31 May 2026 07:22:27 +0800 Subject: [PATCH] fix(runtime): treat cross-realm Error as an error on exit + thrown paths (Ink parity) (#86) app.exit(crossRealmError) resolved waitUntilExit() with the error as a result value, and a component-thrown cross-realm Error was re-wrapped (losing the original) -- both because `instanceof Error` fails across VM realms. Adds Ink's isErrorInput check (instanceof Error || Object.prototype.toString.call(v) === "[object Error]", the realm-independent [object Error] brand) and uses it at the 3 exit() classification sites and the 2 error-boundary normalization sites. A cross-realm Error now rejects (exit) / is preserved (throw), matching Ink; genuine non-Error values still resolve / still wrap. Flips the previously-locked "cross-realm resolves" test to Ink's reject (the behavior was never in ink-divergences.md, so per the repo rule it was an unverified gap, not a sanctioned divergence). Adds component-throw cross-realm + a non-Error-still-wraps guard. Co-authored-by: Claude Opus 4.8 (1M context) --- .../lifecycle/error-handling.test.tsx | 58 +++++++++++++++++++ .../integration/lifecycle/exit.test.tsx | 22 +++---- packages/runtime/src/render.ts | 31 ++++++++-- 3 files changed, 96 insertions(+), 15 deletions(-) diff --git a/packages/runtime-tests/integration/lifecycle/error-handling.test.tsx b/packages/runtime-tests/integration/lifecycle/error-handling.test.tsx index 6af23fa..cfcd345 100644 --- a/packages/runtime-tests/integration/lifecycle/error-handling.test.tsx +++ b/packages/runtime-tests/integration/lifecycle/error-handling.test.tsx @@ -103,6 +103,64 @@ test("error in component triggered after mount routes through exit", async () => await expect(waitUntilExit()).rejects.toThrow("post-mount boom"); }); +test("component-thrown cross-realm Error preserves the original (not re-wrapped)", async () => { + // A cross-realm Error (created in a different VM context) is a genuine Error + // but fails `instanceof Error` because its prototype comes from the other + // realm. The error-boundary path must NOT re-wrap it into + // `new Error(String(foreignError))` (which would yield "Error: boom" and lose + // the original identity). It uses the same isErrorInput brand check as exit(), + // so the ORIGINAL foreign Error rejects waitUntilExit() — matching Ink's + // ErrorBoundary, which rejects with the thrown value itself. + const vm = await import("node:vm"); + const foreignError = vm.runInNewContext("new Error('boom')") as Error; + + const trigger = shallowRef(false); + const App = defineComponent(() => { + return () => { + if (trigger.value) throw foreignError; + return ok; + }; + }); + + const { waitUntilExit, lastFrame } = await render(App); + expect(lastFrame()).toContain("ok"); + + trigger.value = true; + await nextTick(); + await nextTick(); + await Promise.resolve(); + + // Same identity (not a re-wrapped copy) and the original message "boom" + // (NOT "Error: boom" that re-wrapping would produce). + await expect(waitUntilExit()).rejects.toBe(foreignError); + await expect(waitUntilExit()).rejects.toMatchObject({ message: "boom" }); +}); + +test("component-thrown non-Error value is still wrapped into an Error", async () => { + // Guard: a true non-Error throw must still be wrapped into a real Error so the + // exit/ErrorOverview machinery always receives an Error. Only the cross-realm + // Error case is preserved; this case is unchanged. + const trigger = shallowRef(false); + const App = defineComponent(() => { + return () => { + // eslint-disable-next-line no-throw-literal -- exercising a non-Error throw on purpose + if (trigger.value) throw "plain"; + return ok; + }; + }); + + const { waitUntilExit, lastFrame } = await render(App); + expect(lastFrame()).toContain("ok"); + + trigger.value = true; + await nextTick(); + await nextTick(); + await Promise.resolve(); + + await expect(waitUntilExit()).rejects.toBeInstanceOf(Error); + await expect(waitUntilExit()).rejects.toMatchObject({ message: "plain" }); +}); + // --- Ink error validation tests --- test("fail when Box nested inside Text", async () => { diff --git a/packages/runtime-tests/integration/lifecycle/exit.test.tsx b/packages/runtime-tests/integration/lifecycle/exit.test.tsx index 71dcb1d..6b00007 100644 --- a/packages/runtime-tests/integration/lifecycle/exit.test.tsx +++ b/packages/runtime-tests/integration/lifecycle/exit.test.tsx @@ -408,12 +408,13 @@ test("waitUntilExit resolves FIRST exit value when exit is re-entered during unm expect(result).toBe("first"); }); -test("exit with cross-realm Error resolves after stdout write callback", async () => { - // vue-tui uses `instanceof Error` to distinguish errors from result values. - // A cross-realm Error (created in a different VM context) fails the - // instanceof check, so it is treated as a result value and resolves - // rather than rejecting. This differs from Ink which rejects. The test - // verifies the write-callback timing: resolution waits for the barrier. +test("exit with cross-realm Error rejects after stdout write callback", async () => { + // A cross-realm Error (created in a different VM context) is a genuine Error + // but fails `instanceof Error` because its prototype comes from the other + // realm. We classify exit() input with Ink's isErrorInput (instanceof OR the + // [object Error] brand check), so a cross-realm Error REJECTS waitUntilExit() + // — matching Ink. The test also verifies the write-callback timing: + // resolution/rejection waits for the stdout write barrier to flush. const vm = await import("node:vm"); let writeCallbackFired = false; let barrierWriteCallbackFired = false; @@ -450,10 +451,11 @@ test("exit with cross-realm Error resolves after stdout write callback", async ( const { stream: stdin } = makeFakeStdin(); app.mount({ stdout, stdin, stderr, exitOnCtrlC: false }); - // Cross-realm Error fails instanceof check, so exit resolves with the - // error object as a value instead of rejecting. - const result = await app.waitUntilExit(); - expect(result).toBe(foreignError); + // Cross-realm Error is detected via the [object Error] brand check, so exit + // rejects with the foreign error (matching Ink) rather than resolving it as + // a value. + await expect(app.waitUntilExit()).rejects.toBe(foreignError); + await expect(app.waitUntilExit()).rejects.toMatchObject({ message: "boom" }); expect(writeCallbackFired).toBe(true); expect(barrierWriteCallbackFired).toBe(true); }); diff --git a/packages/runtime/src/render.ts b/packages/runtime/src/render.ts index eeacc14..369e692 100644 --- a/packages/runtime/src/render.ts +++ b/packages/runtime/src/render.ts @@ -161,6 +161,20 @@ function shouldClearTerminalForFrame(opts: { // the entry and removes it on teardown; a "no-op" second mount never touches it. const liveInstances = new WeakMap(); +// Classify an exit() input as error-vs-result, matching Ink's isErrorInput +// (ink.tsx:154-159 @ v7.0.4). The plain `instanceof Error` check fails for a +// cross-realm Error — one created in a different VM context (e.g. +// `vm.runInNewContext("new Error()")`) has a prototype from the OTHER realm, so +// it isn't an instance of THIS realm's Error even though it is a genuine Error. +// The `[object Error]` brand (Symbol.toStringTag, not prototype-based) crosses +// realms, so it catches those foreign Errors and they REJECT waitUntilExit() +// instead of being silently swallowed as a resolved result value. Non-Error +// result values (string/number/plain object) brand as e.g. `[object String]`, +// so they still RESOLVE — exactly Ink's contract. +function isErrorInput(value: unknown): value is Error { + return value instanceof Error || Object.prototype.toString.call(value) === "[object Error]"; +} + export function createApp(root: Component, rootProps?: RootProps | null): TuiApp { // exit promise — created at createApp time so waitUntilExit() works even // before mount (it just hangs until mount + exit). @@ -229,7 +243,7 @@ export function createApp(root: Component, rootProps?: RootProps | null): TuiApp // Skipped mount: no stream was ever wired; resolve the exit promise directly // without any write-barrier so the owner's stdout is never touched. if (skippedMount) { - if (pendingExitError instanceof Error) { + if (isErrorInput(pendingExitError)) { exitReject(pendingExitError); } else { exitResolve(pendingExitResult); @@ -241,7 +255,7 @@ export function createApp(root: Component, rootProps?: RootProps | null): TuiApp const hasWritableState = (stdout as any)._writableState !== undefined; const finish = () => { - if (pendingExitError instanceof Error) { + if (isErrorInput(pendingExitError)) { exitReject(pendingExitError); } else { exitResolve(pendingExitResult); @@ -409,7 +423,12 @@ export function createApp(root: Component, rootProps?: RootProps | null): TuiApp const errored = shallowRef(false); onErrorCaptured((err) => { - const e = err instanceof Error ? err : new Error(String(err)); + // Preserve a genuine Error — including a cross-realm one (fails + // `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)); caught.value = err; errored.value = true; // Flush the ErrorOverview frame, then exit @@ -567,7 +586,7 @@ export function createApp(root: Component, rootProps?: RootProps | null): TuiApp // Record the FIRST value/error synchronously (before the deferred // teardown microtask) so a re-entrant exit() — which is blocked above // anyway — and the eventual resolveExit() always settle on this value. - if (errorOrResult instanceof Error) { + if (isErrorInput(errorOrResult)) { pendingExitError = errorOrResult; } else { pendingExitResult = errorOrResult; @@ -980,7 +999,9 @@ export function createApp(root: Component, rootProps?: RootProps | null): TuiApp // async errors in Vue's internal scheduler). The error boundary returns // false to stop propagation, so caught errors won't reach here. baseApp.config.errorHandler = (err) => { - appContext.exit(err instanceof Error ? err : new Error(String(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))); }; // Only listen for resize in interactive mode (matching Ink).