From 09c01031391dc50499903d887ab53b9d5deb6ae3 Mon Sep 17 00:00:00 2001 From: Daniel Stockman Date: Tue, 16 Apr 2019 13:04:42 -0700 Subject: [PATCH] fix(child-process): Centralize `exitCode` translation from string codes into numbers Refs #2031 --- commands/exec/index.js | 15 +------------ .../__tests__/child-process.test.js | 16 +++++++++++++- core/child-process/index.js | 21 +++++++++++++++++++ 3 files changed, 37 insertions(+), 15 deletions(-) diff --git a/commands/exec/index.js b/commands/exec/index.js index 2c50df3aa8..c26a3e532e 100644 --- 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 index cd56cccde5..f57f5b2694 100644 --- 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 index 8290abf568..b097c17182 100644 --- 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;