diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e1ae18414..c8231f1037 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ This is the log of notable changes to EAS CLI and related packages. ### 🐛 Bug fixes +- [eas-cli] Stop `eas workflow:run` from hanging forever when stdin is an open pipe that never delivers data, which is what CI agents hand the process. ([#4261](https://github.com/expo/eas-cli/pull/4261) by [@giaBaoJS](https://github.com/giaBaoJS)) - [build-tools] Preapprove custom URL schemes before opening them in iOS Simulator sessions to avoid the first-use confirmation prompt. ([#4274](https://github.com/expo/eas-cli/pull/4274) by [@szdziedzic](https://github.com/szdziedzic)) - [build-tools] Support downloading iOS simulator archives whose app bundle contents are stored at the archive root. ([#4262](https://github.com/expo/eas-cli/pull/4262) by [@szdziedzic](https://github.com/szdziedzic)) diff --git a/packages/eas-cli/src/commandUtils/workflow/__tests__/utils-test.ts b/packages/eas-cli/src/commandUtils/workflow/__tests__/utils-test.ts index e93555ecee..72b8a2e2ab 100644 --- a/packages/eas-cli/src/commandUtils/workflow/__tests__/utils-test.ts +++ b/packages/eas-cli/src/commandUtils/workflow/__tests__/utils-test.ts @@ -1,6 +1,8 @@ +import { PassThrough } from 'node:stream'; + import { getMockWorkflowRunWithJobsFragment } from '../../../__tests__/commands/utils'; import { fetchRawLogsForCustomJobAsync } from '../fetchLogs'; -import { infoForActiveWorkflowRunAsync } from '../utils'; +import { infoForActiveWorkflowRunAsync, maybeReadStdinAsync } from '../utils'; import { WorkflowJobStatus } from '../../../graphql/generated'; jest.mock('../fetchLogs'); @@ -33,3 +35,107 @@ describe('workflow utils', () => { expect(output).not.toContain('step-id-1'); }); }); + +const DID_NOT_SETTLE = 'maybeReadStdinAsync() did not settle'; + +/** + * Resolves to a sentinel value instead of hanging, so that a promise which never settles fails + * an assertion with a readable message rather than blowing the whole test file's timeout. + */ +async function settleWithinAsync( + promise: Promise, + ms: number +): Promise { + let timeout: NodeJS.Timeout | undefined; + const guard = new Promise(resolve => { + timeout = setTimeout(() => { + resolve(DID_NOT_SETTLE); + }, ms); + }); + + try { + return await Promise.race([promise, guard]); + } finally { + clearTimeout(timeout); + } +} + +function mockProcessStdin(stream: PassThrough & { isTTY?: boolean }): () => void { + const original = Object.getOwnPropertyDescriptor(process, 'stdin')!; + Object.defineProperty(process, 'stdin', { value: stream, configurable: true }); + return () => { + Object.defineProperty(process, 'stdin', original); + }; +} + +describe('maybeReadStdinAsync', () => { + let restoreProcessStdin: (() => void) | undefined; + + afterEach(() => { + restoreProcessStdin?.(); + restoreProcessStdin = undefined; + }); + + test('returns null when stdin is a TTY', async () => { + const stdin: PassThrough & { isTTY?: boolean } = new PassThrough(); + stdin.isTTY = true; + restoreProcessStdin = mockProcessStdin(stdin); + + await expect(settleWithinAsync(maybeReadStdinAsync(), 5000)).resolves.toBeNull(); + }); + + test('returns null when stdin is an open pipe that never delivers data nor ends', async () => { + // This is what a CI agent hands the process: stdin is not a TTY, but nothing is ever written + // to it and nobody closes it, so 'end' never fires. See https://github.com/expo/eas-cli/issues/3164. + restoreProcessStdin = mockProcessStdin(new PassThrough()); + + await expect(settleWithinAsync(maybeReadStdinAsync(), 5000)).resolves.toBeNull(); + }, 20000); + + test('returns null when stdin has already been destroyed', async () => { + const stdin = new PassThrough(); + stdin.destroy(); + restoreProcessStdin = mockProcessStdin(stdin); + + await expect(settleWithinAsync(maybeReadStdinAsync(), 5000)).resolves.toBeNull(); + }, 20000); + + test('reads JSON piped into stdin', async () => { + const stdin = new PassThrough(); + restoreProcessStdin = mockProcessStdin(stdin); + + const stdinPromise = maybeReadStdinAsync(); + stdin.end('{"a":1}\n'); + + await expect(settleWithinAsync(stdinPromise, 5000)).resolves.toBe('{"a":1}'); + }, 20000); + + test('returns null for a pipe that closes without writing anything', async () => { + const stdin = new PassThrough(); + restoreProcessStdin = mockProcessStdin(stdin); + + const stdinPromise = maybeReadStdinAsync(); + stdin.end(); + + await expect(settleWithinAsync(stdinPromise, 5000)).resolves.toBeNull(); + }, 20000); + + test('waits for the whole payload when the writer is slow to finish', async () => { + const stdin = new PassThrough(); + restoreProcessStdin = mockProcessStdin(stdin); + + const stdinPromise = maybeReadStdinAsync(); + stdin.write('{"a":'); + // Longer than the grace period we give the first chunk, to make sure a writer that has + // started talking to us is never cut off mid-payload. + const slowWriteTimeout = setTimeout(() => { + stdin.end('1}'); + }, 2000); + + try { + await expect(settleWithinAsync(stdinPromise, 10000)).resolves.toBe('{"a":1}'); + } finally { + clearTimeout(slowWriteTimeout); + } + }, 20000); +}); diff --git a/packages/eas-cli/src/commandUtils/workflow/utils.ts b/packages/eas-cli/src/commandUtils/workflow/utils.ts index 589d76dd9f..a52bbfff2a 100644 --- a/packages/eas-cli/src/commandUtils/workflow/utils.ts +++ b/packages/eas-cli/src/commandUtils/workflow/utils.ts @@ -270,32 +270,80 @@ export async function fileExistsAsync(filePath: string): Promise { .then(() => true) .catch(() => false); } +/** + * How long we wait for the first chunk of piped stdin before giving up on it. + * + * There is no way to ask a pipe whether anyone will ever write to it, so the only thing we can + * do is wait a little. Anything that is already piped in (`echo '{}' |`, `cat inputs.json |`, + * `< inputs.json`) is available immediately, so a short wait is enough for the supported usage. + */ +const STDIN_FIRST_CHUNK_TIMEOUT_MS = 1000; + export async function maybeReadStdinAsync(): Promise { - // Check if there's data on stdin - if (process.stdin.isTTY) { + const stdin = process.stdin; + + // A TTY is an interactive terminal, there is nothing piped in to read. + if (stdin.isTTY) { + return null; + } + + // A stream that already ended or was destroyed will not emit 'end' again, + // so the promise below would never settle. + if (stdin.readableEnded || stdin.destroyed) { return null; } return await new Promise((resolve, reject) => { let data = ''; + let firstChunkTimeout: NodeJS.Timeout | undefined; - process.stdin.setEncoding('utf8'); + const cleanup = (): void => { + clearTimeout(firstChunkTimeout); + stdin.off('readable', onReadable); + stdin.off('end', onEnd); + stdin.off('error', onError); + // An stdin pipe that is still open keeps the event loop alive even once we have stopped + // reading from it, which would leave the command hanging on exit. + if (typeof stdin.unref === 'function') { + stdin.unref(); + } + }; - process.stdin.on('readable', () => { + const onReadable = (): void => { let chunk; - while ((chunk = process.stdin.read()) !== null) { + while ((chunk = stdin.read()) !== null) { + // Somebody is writing to us, wait for all of it however long it takes. + clearTimeout(firstChunkTimeout); data += chunk; } - }); + }; - process.stdin.on('end', () => { + const onEnd = (): void => { + cleanup(); const trimmedData = data.trim(); resolve(trimmedData || null); - }); + }; - process.stdin.on('error', err => { + const onError = (err: Error): void => { + cleanup(); reject(err); - }); + }; + + stdin.setEncoding('utf8'); + stdin.on('readable', onReadable); + stdin.on('end', onEnd); + stdin.on('error', onError); + + // CI agents (Azure Pipelines, for example) hand the process an stdin pipe that nobody + // writes to and nobody closes. It is not a TTY and it never emits 'end', so without this + // the command would wait forever. See https://github.com/expo/eas-cli/issues/3164. + firstChunkTimeout = setTimeout(() => { + cleanup(); + Log.debug( + `No data on stdin after ${STDIN_FIRST_CHUNK_TIMEOUT_MS}ms, continuing without stdin input.` + ); + resolve(null); + }, STDIN_FIRST_CHUNK_TIMEOUT_MS); }); } export async function showWorkflowStatusAsync(