Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/lib/commands/read-stdin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,17 @@ 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
if (timeout) {
clearTimeout(timeout);
timeout = null;
}
});
};

stream.on('data', onData);

try {
await once(stream, 'end', { signal: controller.signal });
Expand All @@ -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) {
Expand Down
58 changes: 58 additions & 0 deletions test/e2e/commands/stdin-held-open.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
Loading