fix(run-lifecycle): Propagate exit code when execution fails

Fixes #1495

commands/bootstrap/index.js

@@ -181,21 +181,10 @@ class BootstrapCommand extends Command {
 
     const mapPackageWithScript = pkg => {
       if (packagesWithScript.has(pkg)) {
-        return runLifecycle(pkg, stage, this.conf)
-          .then(() => {
-            tracker.silly("lifecycle", "finished", pkg.name);
-            tracker.completeWork(1);
-          })
-          .catch(err => {
-            this.logger.error("bootstrap", `lifecycle '${stage}' errored in '${pkg.name}'`);
-
-            if (err.code) {
-              // log non-lerna error cleanly
-              err.pkg = pkg;
-            }
-
-            throw err;
-          });
+        return runLifecycle(pkg, stage, this.conf).then(() => {
+          tracker.silly("lifecycle", "finished", pkg.name);
+          tracker.completeWork(1);
+        });
       }
     };
 

integration/lerna-publish-lifecycle-errors.test.js

@@ -0,0 +1,67 @@
+"use strict";
+
+const fs = require("fs-extra");
+const path = require("path");
+
+const cliRunner = require("@lerna-test/cli-runner");
+const gitAdd = require("@lerna-test/git-add");
+const gitCommit = require("@lerna-test/git-commit");
+const cloneFixture = require("@lerna-test/clone-fixture")(
+  path.resolve(__dirname, "../commands/publish/__tests__")
+);
+const normalizeTestRoot = require("@lerna-test/normalize-test-root");
+
+const env = {
+  // never actually upload when calling `npm install`
+  npm_config_dry_run: true,
+  // skip npm package validation, none of the stubs are real
+  LERNA_INTEGRATION: "SKIP",
+};
+
+test("lerna publish lifecycle scripts stop on non-zero exit", async () => {
+  const { cwd } = await cloneFixture("lifecycle");
+  const args = ["publish", "minor", "--yes", "--no-verify-registry"];
+
+  const rootManifest = path.join(cwd, "package.json");
+  const json = await fs.readJson(rootManifest);
+
+  json.scripts.preversion = "echo 'bombs away' && exit 123";
+  // console.log(json);
+
+  await fs.writeJson(rootManifest, json);
+  await gitAdd(cwd, rootManifest);
+  await gitCommit(cwd, "update root prepack");
+
+  try {
+    await cliRunner(cwd, env)(...args);
+  } catch (err) {
+    expect(err.code).toBe(123);
+    expect(normalizeTestRoot(err.stdout)).toMatchInlineSnapshot(`
+
+Changes:
+ - package-1: 1.0.0 => 1.1.0
+ - package-2: 1.0.0 => 1.1.0
+
+
+> lifecycle@0.0.0-monorepo preversion __TEST_ROOTDIR__
+> echo 'bombs away' && exit 123
+
+bombs away
+
+`);
+    expect(err.stderr).toMatchInlineSnapshot(`
+lerna notice cli __TEST_VERSION__
+lerna info current version 1.0.0
+lerna info Looking for changed packages since initial commit.
+lerna info auto-confirmed 
+lerna info lifecycle lifecycle@0.0.0-monorepo~preversion: lifecycle@0.0.0-monorepo
+lerna info lifecycle lifecycle@0.0.0-monorepo~preversion: Failed to exec preversion script
+lerna ERR! lifecycle "preversion" errored in "lifecycle", ejecting
+lerna ERR! exited 123 in 'lifecycle'
+lerna ERR! exited 123 in 'lifecycle'
+lerna ERR! exited 123 in 'lifecycle'
+lerna ERR! exited 123 in 'lifecycle'
+
+`);
+  }
+});

utils/run-lifecycle/__tests__/run-lifecycle.test.js

@@ -93,8 +93,15 @@ describe("createRunner", () => {
     expect(npmLifecycle).not.toBeCalled();
   });
 
-  it("logs script error instead of rejecting", async () => {
-    npmLifecycle.mockImplementationOnce(() => Promise.reject(new Error("boom")));
+  it("logs script error and re-throws", async () => {
+    npmLifecycle.mockImplementationOnce(() => {
+      const err = new Error("boom");
+
+      // https://git.io/fAE3f
+      err.errno = 123;
+
+      return Promise.reject(err);
+    });
 
     const pkg = {
       name: "has-script-error",
@@ -103,10 +110,14 @@ describe("createRunner", () => {
       scripts: { prepublishOnly: "exit 1" },
     };
 
-    await runPackageLifecycle(pkg, "prepublishOnly");
+    try {
+      await runPackageLifecycle(pkg, "prepublishOnly");
+    } catch (err) {
+      expect(err.pkg).toBe(pkg);
+      expect(process.exitCode).toBe(123);
+    }
 
     const [errorLog] = loggingOutput("error");
-    expect(errorLog).toMatch("error running prepublishOnly in has-script-error");
-    expect(errorLog).toMatch("boom");
+    expect(errorLog).toMatch('"prepublishOnly" errored in "has-script-error", ejecting');
   });
 });

utils/run-lifecycle/run-lifecycle.js

@@ -34,17 +34,34 @@ function runLifecycle(pkg, stage, opts) {
     failOk: false,
     log,
     unsafePerm: true,
-  }).then(() => pkg);
+  }).then(
+    () => pkg,
+    err => {
+      // error logging has already occurred on stderr, but we need to stop the chain
+      log.error("lifecycle", "%j errored in %j, ejecting", stage, pkg.name);
+
+      // ensure clean logging...
+      err.pkg = pkg;
+
+      // ...propagate the exit code...
+      const exitCode = err.errno;
+
+      // (using the property our yargs.fail() handler expects :P)
+      err.code = exitCode;
+      process.exitCode = exitCode;
+
+      // ...and send it on its merry way
+      throw err;
+    }
+  );
 }
 
 function createRunner(commandOptions) {
   const cfg = npmConf(commandOptions);
 
   return (pkg, stage) => {
     if (pkg.scripts && pkg.scripts[stage]) {
-      return runLifecycle(pkg, stage, cfg).catch(err => {
-        log.error("lifecycle", `error running ${stage} in ${pkg.name}\n`, err.stack || err);
-      });
+      return runLifecycle(pkg, stage, cfg);
     }
 
     return Promise.resolve(pkg);