diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c6f549345..ee4187928b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ This is the log of notable changes to EAS CLI and related packages. ### ๐Ÿ› Bug fixes - [build-tools] Encode macOS simulator session previews with cwebp, because Homebrew's FFmpeg cannot encode WebP. ([#4549](https://github.com/expo/eas-cli/pull/4549) by [@szdziedzic](https://github.com/szdziedzic)) +- [build-tools] Don't crash the build with `kill ESRCH` when a process exits while the `expo doctor`, install dependencies, or install pods timeout is killing it. ([#4561](https://github.com/expo/eas-cli/pull/4561) by [@ahmdshrif](https://github.com/ahmdshrif)) ### ๐Ÿงน Chores diff --git a/packages/build-tools/src/builders/ios.ts b/packages/build-tools/src/builders/ios.ts index d5e2caf562..e18a6adc50 100644 --- a/packages/build-tools/src/builders/ios.ts +++ b/packages/build-tools/src/builders/ios.ts @@ -35,7 +35,7 @@ import { } from '../utils/expoUpdatesEmbedded'; import { Hook, runHookIfPresent } from '../utils/hooks'; import { prepareExecutableAsync } from '../utils/prepareBuildExecutable'; -import { getParentAndDescendantProcessPidsAsync } from '../utils/processes'; +import { getParentAndDescendantProcessPidsAsync, killProcesses } from '../utils/processes'; import { isSourceMapUploadEnabled, resolveIosSourceMapPathAsync } from '../utils/sourceMaps'; const INSTALL_PODS_WARN_TIMEOUT_MS = 15 * 60 * 1000; // 15 minutes @@ -386,9 +386,7 @@ async function runInstallPodsAsync(ctx: BuildContext): Promise { ); const ppid = nullthrows(installPodsSpawnPromise.child.pid); const pids = await getParentAndDescendantProcessPidsAsync(ppid); - pids.forEach(pid => { - process.kill(pid); - }); + killProcesses(pids); ctx.reportError?.('"Install pods" phase takes a very long time', undefined, { extras: { buildId: ctx.env.EAS_BUILD_ID }, }); diff --git a/packages/build-tools/src/common/setup.ts b/packages/build-tools/src/common/setup.ts index 9800789ee0..98bb5ac993 100644 --- a/packages/build-tools/src/common/setup.ts +++ b/packages/build-tools/src/common/setup.ts @@ -28,7 +28,7 @@ import { isAtLeastNpm7Async, shouldUseFrozenLockfile, } from '../utils/packageManager'; -import { getParentAndDescendantProcessPidsAsync } from '../utils/processes'; +import { getParentAndDescendantProcessPidsAsync, killProcesses } from '../utils/processes'; import { readAndLogPackageJson, readEasJsonContents, readPackageJson } from '../utils/project'; import { retryAsync } from '../utils/retry'; @@ -209,9 +209,7 @@ async function runExpoDoctor(ctx: BuildContext): Promise timedOut = true; const ppid = nullthrows(promise.child.pid); const pids = await getParentAndDescendantProcessPidsAsync(ppid); - pids.forEach(pid => { - process.kill(pid); - }); + killProcesses(pids); ctx.reportError?.(`"expo doctor" timed out`, undefined, { extras: { buildId: ctx.env.EAS_BUILD_ID }, }); @@ -272,9 +270,7 @@ async function runInstallDependenciesAsync( ); const ppid = nullthrows(installDependenciesSpawnPromise.child.pid); const pids = await getParentAndDescendantProcessPidsAsync(ppid); - pids.forEach(pid => { - process.kill(pid); - }); + killProcesses(pids); ctx.reportError?.('"Install dependencies" phase takes a very long time', undefined, { extras: { buildId: ctx.env.EAS_BUILD_ID }, }); diff --git a/packages/build-tools/src/utils/__tests__/processes.test.ts b/packages/build-tools/src/utils/__tests__/processes.test.ts index c239701aed..54e5d7ec9d 100644 --- a/packages/build-tools/src/utils/__tests__/processes.test.ts +++ b/packages/build-tools/src/utils/__tests__/processes.test.ts @@ -1,6 +1,7 @@ -import { ChildProcess } from 'node:child_process'; +import { ChildProcess, spawn } from 'node:child_process'; +import { once } from 'node:events'; -import { isChildProcessAlive, isProcessGroupRunning } from '../processes'; +import { isChildProcessAlive, isProcessGroupRunning, killProcesses } from '../processes'; function child(partial: { exitCode: number | null; @@ -67,3 +68,62 @@ describe(isProcessGroupRunning, () => { expect(isProcessGroupRunning(4321)).toBe(true); }); }); + +describe(killProcesses, () => { + function errnoError(code: string): NodeJS.ErrnoException { + const error: NodeJS.ErrnoException = new Error(`kill ${code}`); + error.code = code; + return error; + } + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('kills every pid with the given signal', () => { + const killSpy = jest.spyOn(process, 'kill').mockImplementation(() => true); + + killProcesses([101, 102], 'SIGKILL'); + + expect(killSpy.mock.calls).toEqual([ + [101, 'SIGKILL'], + [102, 'SIGKILL'], + ]); + }); + + it('ignores pids that already exited and keeps killing the rest', () => { + const killSpy = jest.spyOn(process, 'kill').mockImplementation(pid => { + if (pid === 101) { + throw errnoError('ESRCH'); + } + return true; + }); + + expect(() => { + killProcesses([101, 102]); + }).not.toThrow(); + expect(killSpy.mock.calls).toEqual([ + [101, 'SIGTERM'], + [102, 'SIGTERM'], + ]); + }); + + it('does not throw for a real process that has already exited', async () => { + const exited = spawn(process.execPath, ['-e', '']); + await once(exited, 'exit'); + + expect(() => { + killProcesses([exited.pid!]); + }).not.toThrow(); + }); + + it('rethrows other errors', () => { + jest.spyOn(process, 'kill').mockImplementation(() => { + throw errnoError('EPERM'); + }); + + expect(() => { + killProcesses([101]); + }).toThrow('kill EPERM'); + }); +}); diff --git a/packages/build-tools/src/utils/processes.ts b/packages/build-tools/src/utils/processes.ts index 8604eebcd3..4e11f2bee2 100644 --- a/packages/build-tools/src/utils/processes.ts +++ b/packages/build-tools/src/utils/processes.ts @@ -37,6 +37,24 @@ export function killProcessGroup( } } +/** + * Kill each of the given pids. A process tree collected with + * getParentAndDescendantProcessPidsAsync can shrink before the kill lands (e.g. a child + * exits on its own, or exits because its parent was just killed), so pids that no longer + * exist are skipped instead of throwing ESRCH. + */ +export function killProcesses(pids: number[], signal: NodeJS.Signals = 'SIGTERM'): void { + for (const pid of pids) { + try { + process.kill(pid, signal); + } catch (error: any) { + if (error?.code !== 'ESRCH') { + throw error; + } + } + } +} + async function getChildrenPidsAsync(parentPids: number[]): Promise { try { const result = await spawn('pgrep', ['-P', parentPids.join(',')], {