Skip to content

Commit 8c806b1

Browse files
committed
fix: guard inquirer readline and disable conpty for node 24 (replaces patch-package)
Replaces the two patch-package patches with code-owned fixes so the Node 24 behavior ships in compiled source rather than post-install patches. A new bootstrap shim (packages/amplify-cli/src/node24-inquirer-shim.ts) overrides inquirer's base UI.close() to skip rl.pause() on an already-closed readline, avoiding ERR_USE_AFTER_CLOSE on Node 24. It is installed idempotently as the first statement of run() in index.ts, so it ships to customers via the compiled cli-internal lib and runs before any prompt can fire. The e2e asciinema recorder now spawns node-pty with useConpty: false to avoid the Windows ConPTY AttachConsole failure during PTY teardown on Node 24, while keeping ...this.options last so callers can override. The inquirer+7.3.3 and node-pty+1.1.0 patch files are removed since the code fixes replace them. The patch-package devDependency and postinstall hook are intentionally left in place as a harmless no-op (minimal change; husky untouched). --- Prompt: On the node24 branch (PR #14959), replace the two patch-package patches with code-owned fixes: A1 (inquirer bootstrap shim in amplify-cli source) + B (useConpty: false in the e2e harness), and remove both .patch files. Then build/validate, commit, push.
1 parent 829cb83 commit 8c806b1

5 files changed

Lines changed: 71 additions & 38 deletions

File tree

packages/amplify-cli/src/index.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import { deleteOldVersion } from './utils/win-utils';
3535
import { notify } from './version-notifier';
3636
import { getAmplifyVersion } from './extensions/amplify-helpers/get-amplify-version';
3737
import { init as initErrorHandler, handleException, handleUnhandledRejection } from './amplify-exception-handler';
38+
import { installInquirerNode24Shim } from './node24-inquirer-shim';
3839

3940
export { UsageData } from './domain/amplify-usageData';
4041

@@ -64,6 +65,10 @@ const disableCDKDeprecationWarning = () => {
6465
* Command line entry point
6566
*/
6667
export const run = async (startTime: number): Promise<void> => {
68+
// Node 24: guard inquirer's readline teardown against ERR_USE_AFTER_CLOSE.
69+
// Must run before any prompt can fire.
70+
installInquirerNode24Shim();
71+
6772
process.stderr.write(
6873
'\n' +
6974
chalk.yellow(
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/* eslint-disable @typescript-eslint/no-var-requires, global-require */
2+
import type { Interface as ReadlineInterface } from 'readline';
3+
4+
const NODE24_PATCH_FLAG = '__node24Patched';
5+
6+
interface PatchableUI {
7+
rl?: (ReadlineInterface & { closed?: boolean; output?: { end?: () => void; unmute?: () => void } }) | undefined;
8+
activePrompt?: { close?: () => void } | null;
9+
onForceClose?: (() => void) | undefined;
10+
}
11+
12+
type PatchableUIProto = {
13+
close?: (this: PatchableUI) => void;
14+
[NODE24_PATCH_FLAG]?: boolean;
15+
};
16+
17+
/**
18+
* Guards inquirer's base `UI.close()` so it never calls `rl.pause()` on an
19+
* already-closed readline interface. On Node 24 that call throws
20+
* `ERR_USE_AFTER_CLOSE`, which surfaces as a crash when a prompt is torn down;
21+
* on Node <= 22 `pause()` was a silent no-op.
22+
*
23+
* The override is installed once (idempotent via a prototype flag) and replaces
24+
* `baseUI`'s prototype `close`. When the readline is still open it delegates to
25+
* the original implementation; otherwise it replicates the teardown best-effort
26+
* without touching `pause()`. Safe to call multiple times and a no-op if the
27+
* `inquirer/lib/ui/baseUI` module cannot be resolved.
28+
*/
29+
export const installInquirerNode24Shim = (): void => {
30+
let baseUIModule: { default?: unknown } | (new (...args: unknown[]) => unknown);
31+
try {
32+
baseUIModule = require('inquirer/lib/ui/baseUI');
33+
} catch {
34+
return;
35+
}
36+
37+
const UI = ((baseUIModule as { default?: unknown }).default ?? baseUIModule) as (new (...args: unknown[]) => unknown) | undefined;
38+
const proto = UI?.prototype as PatchableUIProto | undefined;
39+
if (!proto || typeof proto.close !== 'function' || proto[NODE24_PATCH_FLAG]) {
40+
return;
41+
}
42+
43+
const originalClose = proto.close;
44+
proto.close = function patchedClose(this: PatchableUI): void {
45+
const rl = this.rl;
46+
if (rl && rl.closed !== true) {
47+
originalClose.call(this);
48+
return;
49+
}
50+
51+
// Readline already closed: mirror baseUI teardown but skip rl.pause() to
52+
// avoid ERR_USE_AFTER_CLOSE on Node 24.
53+
if (rl && this.onForceClose) {
54+
rl.removeListener?.('SIGINT', this.onForceClose);
55+
process.removeListener('exit', this.onForceClose);
56+
}
57+
rl?.output?.unmute?.();
58+
this.activePrompt?.close?.();
59+
rl?.output?.end?.();
60+
rl?.close?.();
61+
};
62+
proto[NODE24_PATCH_FLAG] = true;
63+
};

packages/amplify-e2e-core/src/asciinema-recorder.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ export class Recorder {
6161
shell: true,
6262
// Do not set useConpty. node-pty is smart enough to set it to true only on versions of Windows that support it.
6363
// useConpty: true,
64+
// Node 24 + Windows: force the winpty backend. ConPTY's AttachConsole path can throw during
65+
// PTY teardown on Node 24, so we opt out of ConPTY here to keep e2e recordings stable.
66+
useConpty: false,
6467
...this.options,
6568
});
6669
this.addFrame(this.renderPrompt(this.cwd, this.cmd, this.args));

patches/inquirer+7.3.3.patch

Lines changed: 0 additions & 16 deletions
This file was deleted.

patches/node-pty+1.1.0.patch

Lines changed: 0 additions & 22 deletions
This file was deleted.

0 commit comments

Comments
 (0)