Fix a number of other process.on('exit') bugs similar to #4711

Author: octogonz

apps/heft/src/plugins/NodeServicePlugin.ts

@@ -344,6 +344,8 @@ export default class NodeServicePlugin implements IHeftTaskPlugin {
 
       childProcess.on('exit', (code: number | null, signal: string | null) => {
         try {
+          // Under normal conditions we don't reject the promise here, because 'data' events can continue
+          // to fire as data is flushed, before finally concluding with the 'close' event.
           this._logger.terminal.writeVerboseLine(
             `The service process fired its "exit" event` + this._formatCodeOrSignal(code, signal)
           );

apps/rundown/src/Rundown.ts

@@ -132,7 +132,7 @@ export class Rundown {
     });
 
     await new Promise<void>((resolve, reject) => {
-      childProcess.on('exit', (code: number | null, signal: string | null): void => {
+      childProcess.on('close', (code: number | null, signal: string | null): void => {
         if (code !== 0 && !ignoreExitCode) {
           reject(new Error('Child process terminated with exit code ' + code));
         } else if (!completedNormally) {

apps/rundown/src/launcher.ts

@@ -50,13 +50,6 @@ class Launcher {
     }
   }
 
-  /**
-   * Synchronously delay for the specified time interval.
-   */
-  private static _delayMs(milliseconds: number): void {
-    Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, milliseconds);
-  }
-
   public installHook(): void {
     const realRequire: NodeJS.Require = moduleApi.Module.prototype.require;
 
@@ -113,17 +106,11 @@ class Launcher {
     moduleApi.Module.prototype.require = hookedRequire as NodeJS.Require;
     Launcher._copyProperties(hookedRequire, realRequire);
 
-    process.on('exit', () => {
+    process.on('close', () => {
       this._sendIpcTraceBatch();
       process.send!({
         id: 'done'
       } as IIpcDone);
-
-      // The Node.js "exit" event is synchronous, and the process will terminate as soon as this function returns.
-      // To avoid a race condition, allow some time for IPC messages to be transmitted to the parent process.
-      // TODO: There should be a way to eliminate this delay by intercepting earlier in the shutdown sequence,
-      // but it needs to consider every way that Node.js can exit.
-      Launcher._delayMs(500);
     });
   }
 }

heft-plugins/heft-storybook-plugin/src/StorybookPlugin.ts

@@ -461,7 +461,7 @@ export default class StorybookPlugin implements IHeftTaskPlugin<IStorybookPlugin
         reject(new Error(`Storybook returned error: ${error}`));
       });
 
-      forkedProcess.on('exit', (code: number | null) => {
+      forkedProcess.on('close', (code: number | null) => {
         if (processFinished) {
           return;
         }

libraries/node-core-library/src/Executable.ts

@@ -567,7 +567,7 @@ export class Executable {
       }
     }
 
-    let errorThrown: boolean = false;
+    let errorThrown: Error | undefined = undefined;
     const exitCode: number | null = await new Promise<number | null>(
       (resolve: (result: number | null) => void, reject: (error: Error) => void) => {
         if (encoding) {
@@ -579,13 +579,12 @@ export class Executable {
           });
         }
         childProcess.on('error', (error: Error) => {
-          errorThrown = true;
-          reject(error);
+          // Wait to call reject() until any output is collected
+          errorThrown = error;
         });
-        childProcess.on('exit', (code: number | null) => {
+        childProcess.on('close', (code: number | null) => {
           if (errorThrown) {
-            // We've already rejected the promise
-            return;
+            reject(errorThrown);
           }
           if (code !== 0 && throwOnNonZeroExitCode) {
             reject(new Error(`Process exited with code ${code}`));

libraries/package-deps-hash/src/getRepoState.ts

@@ -292,7 +292,7 @@ async function spawnGitAsync(
 
   const [status] = await once(proc, 'close');
   if (status !== 0) {
-    throw new Error(`git ${args[0]} exited with code ${status}: ${stderr}`);
+    throw new Error(`git ${args[0]} exited with code ${status}:\n${stderr}`);
   }
 
   return stdout;

repo-scripts/repo-toolbox/src/BumpCyclicsAction.ts

@@ -84,7 +84,7 @@ export class BumpCyclicsAction extends CommandLineAction {
       const childProcess: ChildProcess = Executable.spawn('npm', ['view', packageName, 'version']);
       const stdoutBuffer: string[] = [];
       childProcess.stdout!.on('data', (chunk) => stdoutBuffer.push(chunk));
-      childProcess.on('exit', (code: number) => {
+      childProcess.on('close', (code: number) => {
         if (code) {
           reject(new Error(`Exited with ${code}`));
         } else {