diff --git a/commands/bootstrap/index.js b/commands/bootstrap/index.js index 7281b4bb93..515bbc63c5 100644 --- a/commands/bootstrap/index.js +++ b/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); + }); } }; diff --git a/integration/lerna-publish-lifecycle-errors.test.js b/integration/lerna-publish-lifecycle-errors.test.js new file mode 100644 index 0000000000..08c1be1bfd --- /dev/null +++ b/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' + +`); + } +}); diff --git a/utils/run-lifecycle/__tests__/run-lifecycle.test.js b/utils/run-lifecycle/__tests__/run-lifecycle.test.js index dfd5ed38df..28ee6b00c5 100644 --- a/utils/run-lifecycle/__tests__/run-lifecycle.test.js +++ b/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'); }); }); diff --git a/utils/run-lifecycle/run-lifecycle.js b/utils/run-lifecycle/run-lifecycle.js index 3525f44d8e..3736c1f999 100644 --- a/utils/run-lifecycle/run-lifecycle.js +++ b/utils/run-lifecycle/run-lifecycle.js @@ -34,7 +34,26 @@ 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) { @@ -42,9 +61,7 @@ function createRunner(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);