fix: Propagate exit codes from failed executions

This fixes `--no-bail` when used with `lerna exec` or `lerna run`,
ensuring that it still exits with a non-zero code in the event of a
command or script failure.

Refs #1374
Fixes #1653

commands/exec/__tests__/exec-command.test.js

@@ -33,8 +33,12 @@ const execInPackagesStreaming = testDir =>
 
 describe("ExecCommand", () => {
   // TODO: it's very suspicious that mockResolvedValue() doesn't work here
-  ChildProcessUtilities.spawn = jest.fn(() => Promise.resolve());
-  ChildProcessUtilities.spawnStreaming = jest.fn(() => Promise.resolve());
+  ChildProcessUtilities.spawn = jest.fn(() => Promise.resolve({ code: 0 }));
+  ChildProcessUtilities.spawnStreaming = jest.fn(() => Promise.resolve({ code: 0 }));
+
+  afterEach(() => {
+    process.exitCode = undefined;
+  });
 
   describe("in a basic repo", () => {
     it("should complain if invoked without command", async () => {
@@ -50,14 +54,14 @@ describe("ExecCommand", () => {
     });
 
     it("rejects with execution error", async () => {
-      expect.assertions(1);
-
       const testDir = await initFixture("basic");
 
-      ChildProcessUtilities.spawn.mockImplementationOnce(() => {
+      ChildProcessUtilities.spawn.mockImplementationOnce((cmd, args) => {
         const boom = new Error("execution error");
-        boom.code = 1;
-        boom.cmd = "boom";
+
+        boom.failed = true;
+        boom.code = 123;
+        boom.cmd = [cmd].concat(args).join(" ");
 
         throw boom;
       });
@@ -66,18 +70,37 @@ describe("ExecCommand", () => {
         await lernaExec(testDir)("boom");
       } catch (err) {
         expect(err.message).toBe("execution error");
+        expect(err.cmd).toBe("boom");
+        expect(process.exitCode).toBe(123);
       }
     });
 
-    it("should ignore execution errors with --bail=false", async () => {
+    it("should ignore execution errors with --no-bail", async () => {
       const testDir = await initFixture("basic");
 
-      await lernaExec(testDir)("boom", "--no-bail");
+      ChildProcessUtilities.spawn.mockImplementationOnce((cmd, args, { pkg }) => {
+        const boom = new Error(pkg.name);
+
+        boom.failed = true;
+        boom.code = 456;
+        boom.cmd = [cmd].concat(args).join(" ");
+
+        // --no-bail passes { reject: false } to execa, so throwing is inappropriate
+        return Promise.resolve(boom);
+      });
+
+      try {
+        await lernaExec(testDir)("boom", "--no-bail", "--", "--shaka", "--lakka");
+      } catch (err) {
+        expect(err.message).toBe("package-1");
+        expect(err.cmd).toBe("boom --shaka --lakka");
+        expect(process.exitCode).toBe(456);
+      }
 
       expect(ChildProcessUtilities.spawn).toHaveBeenCalledTimes(2);
       expect(ChildProcessUtilities.spawn).lastCalledWith(
         "boom",
-        [],
+        ["--shaka", "--lakka"],
         expect.objectContaining({
           reject: false,
         })

commands/exec/index.js

@@ -45,33 +45,68 @@ class ExecCommand extends Command {
 
     return chain.then(() => {
       this.count = this.filteredPackages.length;
+      this.packagePlural = this.count === 1 ? "package" : "packages";
+      this.joinedCommand = [this.command].concat(this.args).join(" ");
+
       this.batchedPackages = this.toposort
         ? batchPackages(this.filteredPackages, this.options.rejectCycles)
         : [this.filteredPackages];
     });
   }
 
   execute() {
+    this.logger.info(
+      "",
+      "Executing command in %d %s: %j",
+      this.count,
+      this.packagePlural,
+      this.joinedCommand
+    );
+
+    let chain = Promise.resolve();
+
     if (this.options.parallel) {
-      return this.runCommandInPackagesParallel();
+      chain = chain.then(() => this.runCommandInPackagesParallel());
+    } else {
+      chain = chain.then(() => this.runCommandInPackagesBatched());
     }
 
-    const runner = this.options.stream
-      ? pkg => this.runCommandInPackageStreaming(pkg)
-      : pkg => this.runCommandInPackageCapturing(pkg);
+    if (this.bail) {
+      // only the first error is caught
+      chain = chain.catch(err => {
+        process.exitCode = err.code;
+
+        // rethrow to halt chain and log properly
+        throw err;
+      });
+    } else {
+      // detect error (if any) from collected results
+      chain = chain.then(results => {
+        /* 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 => result.code);
+          const exitCode = Math.max(...codes, 1);
+
+          this.logger.error("", "Received non-zero exit code %d during execution", exitCode);
+          process.exitCode = exitCode;
+        }
+      });
+    }
 
-    return runParallelBatches(this.batchedPackages, this.concurrency, runner).then(() => {
+    return chain.then(() => {
       this.logger.success(
         "exec",
         "Executed command in %d %s: %j",
         this.count,
-        this.count === 1 ? "package" : "packages",
-        [this.command].concat(this.args).join(" ")
+        this.packagePlural,
+        this.joinedCommand
       );
     });
   }
 
   getOpts(pkg) {
+    // these options are passed _directly_ to execa
     return {
       cwd: pkg.location,
       shell: true,
@@ -85,15 +120,17 @@ class ExecCommand extends Command {
     };
   }
 
-  runCommandInPackagesParallel() {
-    this.logger.info(
-      "exec",
-      "in %d %s: %j",
-      this.count,
-      this.count === 1 ? "package" : "packages",
-      [this.command].concat(this.args).join(" ")
+  runCommandInPackagesBatched() {
+    const runner = this.options.stream
+      ? pkg => this.runCommandInPackageStreaming(pkg)
+      : pkg => this.runCommandInPackageCapturing(pkg);
+
+    return runParallelBatches(this.batchedPackages, this.concurrency, runner).then(batchedResults =>
+      batchedResults.reduce((arr, batch) => arr.concat(batch), [])
     );
+  }
 
+  runCommandInPackagesParallel() {
     return Promise.all(this.filteredPackages.map(pkg => this.runCommandInPackageStreaming(pkg)));
   }
 

commands/run/__tests__/run-command.test.js

@@ -33,8 +33,12 @@ const ranInPackagesStreaming = testDir =>
   }, []);
 
 describe("RunCommand", () => {
-  npmRunScript.mockImplementation((script, { pkg }) => Promise.resolve({ stdout: pkg.name }));
-  npmRunScript.stream.mockImplementation(() => Promise.resolve());
+  npmRunScript.mockImplementation((script, { pkg }) => Promise.resolve({ code: 0, stdout: pkg.name }));
+  npmRunScript.stream.mockImplementation(() => Promise.resolve({ code: 0 }));
+
+  afterEach(() => {
+    process.exitCode = undefined;
+  });
 
   describe("in a basic repo", () => {
     it("should complain if invoked with an empty script", async () => {
@@ -171,8 +175,16 @@ describe("RunCommand", () => {
     });
 
     it("reports script errors with early exit", async () => {
-      expect.assertions(2);
-      npmRunScript.mockImplementationOnce((script, { pkg }) => Promise.reject(new Error(pkg.name)));
+      expect.assertions(3);
+
+      npmRunScript.mockImplementationOnce((script, { pkg }) => {
+        const err = new Error(pkg.name);
+
+        err.failed = true;
+        err.code = 123;
+
+        return Promise.reject(err);
+      });
 
       const testDir = await initFixture("basic");
 
@@ -181,8 +193,28 @@ describe("RunCommand", () => {
       } catch (err) {
         expect(err.message).toMatch("package-1");
         expect(err.message).not.toMatch("package-2");
+        expect(process.exitCode).toBe(123);
       }
     });
+
+    it("propagates non-zero exit codes with --no-bail", async () => {
+      npmRunScript.mockImplementationOnce((script, { pkg }) => {
+        const err = new Error(pkg.name);
+
+        err.failed = true;
+        err.code = 456;
+        err.stdout = pkg.name;
+
+        return Promise.resolve(err);
+      });
+
+      const testDir = await initFixture("basic");
+
+      await lernaRun(testDir)("my-script", "--no-bail");
+
+      expect(process.exitCode).toBe(456);
+      expect(output.logged().split("\n")).toEqual(["package-1", "package-3"]);
+    });
   });
 
   describe("with --include-filtered-dependencies", () => {

commands/run/index.js

@@ -48,6 +48,8 @@ class RunCommand extends Command {
 
     return chain.then(() => {
       this.count = this.packagesWithScript.length;
+      this.packagePlural = this.count === 1 ? "package" : "packages";
+      this.joinedCommand = [this.npmClient, "run", this.script].concat(this.args).join(" ");
 
       if (!this.count) {
         this.logger.success("run", `No packages found with the lifecycle script '${script}'`);
@@ -63,6 +65,14 @@ class RunCommand extends Command {
   }
 
   execute() {
+    this.logger.info(
+      "",
+      "Executing command in %d %s: %j",
+      this.count,
+      this.packagePlural,
+      this.joinedCommand
+    );
+
     let chain = Promise.resolve();
 
     if (this.options.parallel) {
@@ -71,19 +81,43 @@ class RunCommand extends Command {
       chain = chain.then(() => this.runScriptInPackagesBatched());
     }
 
+    if (this.bail) {
+      // only the first error is caught
+      chain = chain.catch(err => {
+        process.exitCode = err.code;
+
+        // rethrow to halt chain and log properly
+        throw err;
+      });
+    } else {
+      // detect error (if any) from collected results
+      chain = chain.then(results => {
+        /* 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 => result.code);
+          const exitCode = Math.max(...codes, 1);
+
+          this.logger.error("", "Received non-zero exit code %d during execution", exitCode);
+          process.exitCode = exitCode;
+        }
+      });
+    }
+
     return chain.then(() => {
       this.logger.success(
         "run",
         "Ran npm script '%s' in %d %s:",
         this.script,
         this.count,
-        this.count === 1 ? "package" : "packages"
+        this.packagePlural
       );
       this.logger.success("", this.packagesWithScript.map(pkg => `- ${pkg.name}`).join("\n"));
     });
   }
 
   getOpts(pkg) {
+    // these options are NOT passed directly to execa, they are composed in npm-run-script
     return {
       args: this.args,
       npmClient: this.npmClient,
@@ -98,19 +132,12 @@ class RunCommand extends Command {
       ? pkg => this.runScriptInPackageStreaming(pkg)
       : pkg => this.runScriptInPackageCapturing(pkg);
 
-    return runParallelBatches(this.batchedPackages, this.concurrency, runner);
+    return runParallelBatches(this.batchedPackages, this.concurrency, runner).then(batchedResults =>
+      batchedResults.reduce((arr, batch) => arr.concat(batch), [])
+    );
   }
 
   runScriptInPackagesParallel() {
-    this.logger.info(
-      "run",
-      "in %d %s: %s run %s",
-      this.count,
-      this.count === 1 ? "package" : "packages",
-      this.npmClient,
-      [this.script].concat(this.args).join(" ")
-    );
-
     return pMap(this.packagesWithScript, pkg => this.runScriptInPackageStreaming(pkg));
   }
 
@@ -121,6 +148,8 @@ class RunCommand extends Command {
   runScriptInPackageCapturing(pkg) {
     return npmRunScript(this.script, this.getOpts(pkg)).then(result => {
       output(result.stdout);
+
+      return result;
     });
   }
 }

integration/__fixtures__/lerna-run/packages/package-1/package.json

@@ -6,7 +6,7 @@
   },
   "scripts": {
     "prefail": "echo package-1",
-    "fail": "exit 1",
+    "fail": "exit 100",
     "my-script": "echo package-1",
     "test": "echo package-1"
   }

integration/lerna-exec.test.js

@@ -152,11 +152,24 @@ package-2
 
   test("--no-bail", async () => {
     const cwd = await initFixture("lerna-exec");
-    const args = ["exec", "--no-bail", "--concurrency=1", "--", "npm", "run", "fail-or-succeed"];
+    const args = ["exec", "--no-bail", "--concurrency=1", "--", "npm", "run", "fail-or-succeed", "--silent"];
 
-    const { stdout, stderr } = await cliRunner(cwd)(...args);
-    expect(stderr).toMatch("Failed at the package-1@1.0.0 fail-or-succeed script");
-    expect(stdout).toMatch("failure!");
-    expect(stdout).toMatch("success!");
+    try {
+      await cliRunner(cwd)(...args);
+    } catch (err) {
+      expect(err.stderr).toMatchInlineSnapshot(`
+lerna notice cli __TEST_VERSION__
+lerna info Executing command in 2 packages: "npm run fail-or-succeed --silent"
+lerna ERR! Received non-zero exit code 1 during execution
+lerna success exec Executed command in 2 packages: "npm run fail-or-succeed --silent"
+
+`);
+      expect(err.stdout).toMatchInlineSnapshot(`
+failure!
+success!
+
+`);
+      expect(err.code).toBe(1);
+    }
   });
 });