fix(child-process): Centralize exitCode translation from string cod…

…es into numbers Refs #2031
lerna · Apr 17, 2019 · 09c0103 · 09c0103
1 parent c599c64
commit 09c0103
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 15 deletions.
diff --git a/commands/exec/index.js b/commands/exec/index.js
@@ -1,7 +1,5 @@
 "use strict";
 
-const os = require("os");
-
 const ChildProcessUtilities = require("@lerna/child-process");
 const Command = require("@lerna/command");
 const batchPackages = require("@lerna/batch-packages");
@@ -87,18 +85,7 @@ class ExecCommand extends Command {
         /* istanbul ignore else */
         if (results.some(result => result.failed)) {
           // propagate "highest" error code, it's probably the most useful
-          const codes = results
-            .filter(result => result.failed)
-            .map(result => {
-              switch (typeof result.code) {
-                case "number":
-                  return result.code;
-                case "string":
-                  return os.constants.errno[result.code];
-                default:
-                  throw new TypeError("Received unexpected exit code value");
-              }
-            });
+          const codes = results.filter(result => result.failed).map(result => result.code);
           const exitCode = Math.max(...codes, 1);
 
           this.logger.error("", "Received non-zero exit code %d during execution", exitCode);

diff --git a/core/child-process/__tests__/child-process.test.js b/core/child-process/__tests__/child-process.test.js
@@ -1,5 +1,7 @@
 "use strict";
 
+const os = require("os");
+
 // file under test
 const ChildProcessUtilities = require("..");
 
@@ -51,7 +53,7 @@ describe("ChildProcessUtilities", () => {
           { pkg: { name: "hamilton" } }
         );
       } catch (err) {
-        expect(err.code).toBe("ENOENT");
+        expect(err.code).toBe(os.constants.errno.ENOENT);
         expect(err.pkg).toEqual({ name: "hamilton" });
       }
     });
@@ -66,5 +68,17 @@ describe("ChildProcessUtilities", () => {
       expect(code).toBe(0);
       expect(signal).toBe(null);
     });
+
+    it("decorates opts.pkg on error if caught", async () => {
+      try {
+        await ChildProcessUtilities.spawn("exit", ["123"], {
+          pkg: { name: "shelled" },
+          shell: true,
+        });
+      } catch (err) {
+        expect(err.code).toBe(123);
+        expect(err.pkg).toEqual({ name: "shelled" });
+      }
+    });
   });
 });
diff --git a/core/child-process/index.js b/core/child-process/index.js
@@ -1,5 +1,6 @@
 "use strict";
 
+const os = require("os");
 const chalk = require("chalk");
 const execa = require("execa");
 const logTransformer = require("strong-log-transformer");
@@ -62,6 +63,22 @@ function getChildProcessCount() {
   return children;
 }
 
+function getExitCode(result) {
+  // https://nodejs.org/docs/latest-v6.x/api/child_process.html#child_process_event_close
+  if (typeof result.code === "number") {
+    return result.code;
+  }
+
+  // https://nodejs.org/docs/latest-v6.x/api/errors.html#errors_error_code
+  // istanbul ignore else
+  if (typeof result.code === "string") {
+    return os.constants.errno[result.code];
+  }
+
+  // istanbul ignore next: extremely weird
+  throw new TypeError(`Received unexpected exit code value ${JSON.stringify(result.code)}`);
+}
+
 function spawnProcess(command, args, opts) {
   children += 1;
 
@@ -90,6 +107,9 @@ function wrapError(spawned) {
     return spawned.catch(err => {
       // istanbul ignore else
       if (err.code) {
+        // ensure code is always a number
+        err.code = getExitCode(err);
+
         // log non-lerna error cleanly
         err.pkg = spawned.pkg;
       }
@@ -106,3 +126,4 @@ exports.execSync = execSync;
 exports.spawn = spawn;
 exports.spawnStreaming = spawnStreaming;
 exports.getChildProcessCount = getChildProcessCount;
+exports.getExitCode = getExitCode;