fix(runtime): guard teardown cursor/kitty restore writes against a destroyed stdout (Ink parity) (#127)
Two more teardown stdout writes (besides the paste-disable fixed in #126) were gated only on isTTY, which stays cached-truthy after destroy()/end() — so a teardown on an already-gone stdout threw ERR_STREAM_DESTROYED: log-update's show/hide-cursor restore and kitty-keyboard's async disable-kitty write. Both now also require !destroyed && !writableEnded, matching Ink's canWriteToStdout (App.tsx:620, ink.tsx:792). The writeBestEffort-routed restores (alt-screen exit, non-interactive last frame, final commit) were already safe. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,212 @@
|
||||
/**
|
||||
* Item 2.5b — every teardown stdout write must be skipped when stdout is already
|
||||
* destroyed/ended, not just gated on `isTTY`.
|
||||
*
|
||||
* On teardown vue-tui restores terminal state by writing escapes to stdout
|
||||
* (show-cursor `\x1b[?25h` via the frame writer's `done()`, alt-screen exit,
|
||||
* disable-kitty `\x1b[<u`). Some of these were gated ONLY on `isTTY` — which
|
||||
* stays cached-truthy after `destroy()`/`end()` — with NO writability check, so
|
||||
* on a teardown where stdout is already gone the unguarded `.write()` hits a
|
||||
* dead stream (ERR_STREAM_DESTROYED). Ink guards the analogous cursor-show and
|
||||
* disable-kitty writes on `canWriteToStdout = !destroyed && !writableEnded`
|
||||
* (App.tsx:620-624 for the cursor show; ink.tsx:792-795 for disable-kitty).
|
||||
*
|
||||
* The paste-disable site was fixed in PR #126; this covers the SAME bug class at
|
||||
* the OTHER teardown sites. We use a recording TTY double that RECORDS every
|
||||
* restore-escape write attempt and whether the stream was already dead at the
|
||||
* time, so the test asserts the runtime never even ATTEMPTS the write on a dead
|
||||
* stdout (the observable contract), rather than relying on a call site swallowing
|
||||
* the throw.
|
||||
*/
|
||||
import { PassThrough } from "node:stream";
|
||||
import { defineComponent, nextTick } from "vue";
|
||||
import { describe, test, expect } from "vite-plus/test";
|
||||
import { createApp, Text, useCursor } from "@vue-tui/runtime";
|
||||
|
||||
const SHOW_CURSOR = "\x1b[?25h";
|
||||
const DISABLE_KITTY = "\x1b[<u";
|
||||
|
||||
// A TTY-ish stdout that mirrors a real terminal torn down underneath us: after
|
||||
// hardDestroy() `isTTY` stays cached-truthy while the stream is no longer
|
||||
// writable. It RECORDS every restore-escape write attempt made while dead.
|
||||
function makeRecordingTtyStream(): NodeJS.WriteStream & {
|
||||
chunks: string[];
|
||||
showCursorWhileDead: number;
|
||||
disableKittyWhileDead: number;
|
||||
hardDestroy(): void;
|
||||
} {
|
||||
const inner = new PassThrough();
|
||||
const s = inner as unknown as NodeJS.WriteStream & {
|
||||
chunks: string[];
|
||||
showCursorWhileDead: number;
|
||||
disableKittyWhileDead: number;
|
||||
hardDestroy(): void;
|
||||
};
|
||||
let dead = false;
|
||||
s.chunks = [];
|
||||
s.showCursorWhileDead = 0;
|
||||
s.disableKittyWhileDead = 0;
|
||||
inner.on("data", (chunk: Buffer) => s.chunks.push(chunk.toString()));
|
||||
|
||||
const realWrite = inner.write.bind(inner);
|
||||
s.write = ((data: string | Uint8Array, ...rest: unknown[]) => {
|
||||
const str = String(data);
|
||||
if (dead && (str.includes(SHOW_CURSOR) || str.includes(DISABLE_KITTY))) {
|
||||
// The bug: a restore-escape write attempted on a destroyed stream. In a
|
||||
// real terminal this throws ERR_STREAM_DESTROYED; reproduce that here.
|
||||
if (str.includes(SHOW_CURSOR)) s.showCursorWhileDead++;
|
||||
if (str.includes(DISABLE_KITTY)) s.disableKittyWhileDead++;
|
||||
const err = new Error("write after destroy") as NodeJS.ErrnoException;
|
||||
err.code = "ERR_STREAM_DESTROYED";
|
||||
throw err;
|
||||
}
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
return (realWrite as any)(str, ...rest);
|
||||
}) as NodeJS.WriteStream["write"];
|
||||
|
||||
s.hardDestroy = () => {
|
||||
dead = true;
|
||||
inner.destroy();
|
||||
};
|
||||
|
||||
Object.assign(s, { columns: 80, rows: 24, isTTY: true });
|
||||
return s;
|
||||
}
|
||||
|
||||
function makeFakeStdin(): NodeJS.ReadStream {
|
||||
const s = new PassThrough() as unknown as NodeJS.ReadStream;
|
||||
Object.assign(s, {
|
||||
isTTY: true,
|
||||
setRawMode() {
|
||||
return s;
|
||||
},
|
||||
setEncoding() {
|
||||
return s;
|
||||
},
|
||||
});
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(s as any).ref = () => {};
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(s as any).unref = () => {};
|
||||
return s;
|
||||
}
|
||||
|
||||
const CursorApp = defineComponent(() => {
|
||||
// useCursor makes the frame writer carry a live cursor, but the show-cursor
|
||||
// restore at done() fires for ANY interactive app — the cursor was hidden at
|
||||
// mount and the frame writer's done() shows it again on teardown.
|
||||
useCursor();
|
||||
return () => <Text>cursor</Text>;
|
||||
});
|
||||
|
||||
const PlainApp = defineComponent(() => () => <Text>plain</Text>);
|
||||
|
||||
describe("teardown stdout writes on destroyed stdout", () => {
|
||||
test("teardown skips the show-cursor write (frame writer done) when stdout was destroyed", async () => {
|
||||
const stdout = makeRecordingTtyStream();
|
||||
const stderr = makeRecordingTtyStream();
|
||||
const stdin = makeFakeStdin();
|
||||
|
||||
const app = createApp(PlainApp);
|
||||
app.mount({ stdout, stdin, stderr, debug: false, exitOnCtrlC: false });
|
||||
|
||||
// Let the initial render settle (cursor hidden at mount).
|
||||
await new Promise<void>((r) => setTimeout(r, 60));
|
||||
expect(stdout.chunks.join("")).toContain("\x1b[?25l");
|
||||
|
||||
// Terminal torn down underneath us BEFORE teardown. isTTY stays truthy.
|
||||
stdout.hardDestroy();
|
||||
|
||||
// Teardown must not even ATTEMPT the show-cursor write on a dead stdout
|
||||
// (via mountedWriter.done() -> log-update showCursor), and must not throw.
|
||||
expect(() => app.unmount()).not.toThrow();
|
||||
expect(
|
||||
stdout.showCursorWhileDead,
|
||||
"no show-cursor (\\x1b[?25h) write may be attempted on a destroyed stdout",
|
||||
).toBe(0);
|
||||
});
|
||||
|
||||
test("teardown skips the show-cursor write for a useCursor app when stdout was destroyed", async () => {
|
||||
const stdout = makeRecordingTtyStream();
|
||||
const stderr = makeRecordingTtyStream();
|
||||
const stdin = makeFakeStdin();
|
||||
|
||||
const app = createApp(CursorApp);
|
||||
app.mount({ stdout, stdin, stderr, debug: false, exitOnCtrlC: false });
|
||||
|
||||
await new Promise<void>((r) => setTimeout(r, 60));
|
||||
stdout.hardDestroy();
|
||||
|
||||
expect(() => app.unmount()).not.toThrow();
|
||||
expect(stdout.showCursorWhileDead).toBe(0);
|
||||
});
|
||||
|
||||
test("teardown skips the disable-kitty write when stdout was destroyed", async () => {
|
||||
const stdout = makeRecordingTtyStream();
|
||||
const stderr = makeRecordingTtyStream();
|
||||
const stdin = makeFakeStdin();
|
||||
|
||||
const app = createApp(PlainApp);
|
||||
app.mount({
|
||||
stdout,
|
||||
stdin,
|
||||
stderr,
|
||||
debug: false,
|
||||
exitOnCtrlC: false,
|
||||
// Force kitty enabled so dispose() attempts the disable-kitty escape.
|
||||
kittyKeyboard: { mode: "enabled" },
|
||||
});
|
||||
|
||||
await new Promise<void>((r) => setTimeout(r, 60));
|
||||
// Kitty enable escape should have gone out on the live stream.
|
||||
expect(stdout.chunks.join("")).toContain("\x1b[>");
|
||||
|
||||
stdout.hardDestroy();
|
||||
|
||||
expect(() => app.unmount()).not.toThrow();
|
||||
expect(
|
||||
stdout.disableKittyWhileDead,
|
||||
"no disable-kitty (\\x1b[<u) write may be attempted on a destroyed stdout",
|
||||
).toBe(0);
|
||||
});
|
||||
|
||||
test("live stdout still receives the show-cursor escape at teardown", async () => {
|
||||
const stdout = makeRecordingTtyStream();
|
||||
const stderr = makeRecordingTtyStream();
|
||||
const stdin = makeFakeStdin();
|
||||
|
||||
const app = createApp(PlainApp);
|
||||
app.mount({ stdout, stdin, stderr, debug: false, exitOnCtrlC: false });
|
||||
|
||||
await new Promise<void>((r) => setTimeout(r, 60));
|
||||
|
||||
// Do NOT destroy: a live stdout must still get the cursor restored.
|
||||
app.unmount();
|
||||
await nextTick();
|
||||
|
||||
expect(stdout.chunks.join("")).toContain(SHOW_CURSOR);
|
||||
});
|
||||
|
||||
test("live stdout still receives the disable-kitty escape at teardown", async () => {
|
||||
const stdout = makeRecordingTtyStream();
|
||||
const stderr = makeRecordingTtyStream();
|
||||
const stdin = makeFakeStdin();
|
||||
|
||||
const app = createApp(PlainApp);
|
||||
app.mount({
|
||||
stdout,
|
||||
stdin,
|
||||
stderr,
|
||||
debug: false,
|
||||
exitOnCtrlC: false,
|
||||
kittyKeyboard: { mode: "enabled" },
|
||||
});
|
||||
|
||||
await new Promise<void>((r) => setTimeout(r, 60));
|
||||
|
||||
app.unmount();
|
||||
await nextTick();
|
||||
|
||||
expect(stdout.chunks.join("")).toContain(DISABLE_KITTY);
|
||||
});
|
||||
});
|
||||
@@ -221,7 +221,12 @@ export function createKittyKeyboardController(
|
||||
} catch {
|
||||
// Best-effort restore during abrupt shutdown.
|
||||
}
|
||||
} else {
|
||||
} else if (!stdout.destroyed && !(stdout as { writableEnded?: boolean }).writableEnded) {
|
||||
// Skip the disable-kitty write on a destroyed/ended stdout: `isTTY`
|
||||
// stays cached-truthy after destroy()/end(), so an unguarded write on
|
||||
// a teardown where stdout is already gone throws ERR_STREAM_DESTROYED.
|
||||
// Mirror Ink's `if (canWriteToStdout) writeBestEffort(stdout,
|
||||
// '[<u')` (ink.tsx:792-795).
|
||||
stdout.write("\x1b[<u");
|
||||
}
|
||||
enabled = false;
|
||||
|
||||
@@ -35,15 +35,24 @@ const visibleLineCount = (lines: string[], str: string): number =>
|
||||
// `isTTY`, so we read it off the runtime object (WriteStream sets it).
|
||||
const isTtyStream = (stream: Writable): boolean => Boolean((stream as { isTTY?: boolean }).isTTY);
|
||||
|
||||
// The show-cursor restore at done() runs on the teardown path, where stdout may
|
||||
// already be destroyed/ended. `isTTY` stays cached-truthy after destroy()/end(),
|
||||
// so gating cursor writes on isTTY alone throws ERR_STREAM_DESTROYED on a
|
||||
// teardown where the terminal is already gone. Mirror Ink's `canWriteToStdout =
|
||||
// !destroyed && !writableEnded` guard (App.tsx:620-624, the cursor-show on
|
||||
// unmount) so a TTY-gated cursor write is also skipped on a dead stream.
|
||||
const canWriteToStream = (stream: Writable): boolean =>
|
||||
!stream.destroyed && !(stream as { writableEnded?: boolean }).writableEnded;
|
||||
|
||||
const hideCursor = (stream: Writable): void => {
|
||||
if (!isTtyStream(stream)) {
|
||||
if (!isTtyStream(stream) || !canWriteToStream(stream)) {
|
||||
return;
|
||||
}
|
||||
stream.write(hideCursorEscape);
|
||||
};
|
||||
|
||||
const showCursor = (stream: Writable): void => {
|
||||
if (!isTtyStream(stream)) {
|
||||
if (!isTtyStream(stream) || !canWriteToStream(stream)) {
|
||||
return;
|
||||
}
|
||||
stream.write(showCursorEscape);
|
||||
|
||||
Reference in New Issue
Block a user