diff --git a/src/lib/commands/read-stdin.ts b/src/lib/commands/read-stdin.ts index e131089ad..2ef62b391 100644 --- a/src/lib/commands/read-stdin.ts +++ b/src/lib/commands/read-stdin.ts @@ -23,7 +23,7 @@ export async function readStdin() { }, waitDelay).unref(); } - stream.on('data', (chunk) => { + const onData = (chunk: Buffer) => { bufferChunks.push(chunk); // If we got some data already, we can clear the timeout, as we will get more @@ -31,7 +31,9 @@ export async function readStdin() { clearTimeout(timeout); timeout = null; } - }); + }; + + stream.on('data', onData); try { await once(stream, 'end', { signal: controller.signal }); @@ -41,6 +43,14 @@ export async function readStdin() { if (casted.name === 'AbortError') { return; } + } finally { + // Stop reading from stdin so its open handle can't keep the event loop (and + // the CLI) alive after the command finishes (#1206). This only helps when the + // await above settles ('end' or the no-data abort). A writer that sends data + // but never closes stdin still hangs up there; that needs the lazy stdin + // reading discussed in #1206. + stream.off('data', onData); + stream.pause(); } if (timeout) { diff --git a/test/e2e/commands/stdin-held-open.test.ts b/test/e2e/commands/stdin-held-open.test.ts new file mode 100644 index 000000000..11d58db37 --- /dev/null +++ b/test/e2e/commands/stdin-held-open.test.ts @@ -0,0 +1,58 @@ +import { mkdir, rm } from 'node:fs/promises'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { execa } from 'execa'; + +import { TestTmpRoot } from '../__helpers__/tmp.js'; + +const DistApify = fileURLToPath(new URL('../../../dist/apify.js', import.meta.url)); + +// How long the CLI gets to exit on its own. The command itself finishes in well +// under a second; the broken behavior never exits at all, so any finite deadline +// separates the two. Generous to absorb slow CI runners. +const EXIT_DEADLINE_MS = 15_000; + +describe('[e2e] stdin held open (#1206)', () => { + const emptyDir = path.join(TestTmpRoot, 'stdin-held-open'); + + beforeAll(async () => { + await rm(emptyDir, { recursive: true, force: true }); + await mkdir(emptyDir, { recursive: true }); + }); + + afterAll(async () => { + await rm(emptyDir, { recursive: true, force: true }); + }); + + it('exits on its own when stdin is a pipe that never closes', async () => { + // The runCli helper cannot express this scenario: it either ignores stdin or + // writes input and closes it. execa with stdin: 'pipe' and no `input` keeps + // the child's stdin open for the child's whole lifetime (verified on execa 9), + // which is what spawned subprocesses (CI runners, agent shells) see. Before + // the fix, the startup stdin read left process.stdin flowing with a data + // listener attached, so the CLI printed all its output but never exited. + const result = await execa('node', [DistApify, 'run'], { + cwd: emptyDir, + reject: false, + timeout: EXIT_DEADLINE_MS, + stdin: 'pipe', + env: { + APIFY_CLI_DISABLE_TELEMETRY: '1', + APIFY_CLI_SKIP_UPDATE_CHECK: '1', + APIFY_DISABLE_KEYRING: '1', + }, + }); + + // `timedOut` is the regression signal: the broken build completes the command + // (same stderr) but never exits, so execa kills it at the deadline. + expect(result.timedOut, `stderr: ${result.stderr}`).toBe(false); + + // Sanity: the command really ran and failed naturally (an empty dir is not an + // actor project). Keep this on `apify run`: --version, --help, and unknown + // commands all call process.exit(), which exits even with a leaked stdin + // handle and would mask the regression. + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain('Actor is of an unknown format'); + }); +});