[build-tools] Ignore already-exited processes when killing timed-out phases

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
 

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<Ios.Job>): Promise<void> {
       );
       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 },
       });

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<TJob extends Job>(ctx: BuildContext<TJob>): 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<TJob extends Job>(
       );
       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 },
       });

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');
+  });
+});

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<number[]> {
   try {
     const result = await spawn('pgrep', ['-P', parentPids.join(',')], {