Repository: semantic-release/semantic-release
Pull request #491 (2017-10-29 06:12:07)

Log error properties and check SemanticReleaseError with name
`semantic-release` now log all information included in unexpected errors.
In order to verify if an error is a `SemanticReleaseError` we now use the `name` property rather than the instance type. The test will work properly if a plugin use a different version of `@semantic-release/error` than `semantic-release`.

We should probably deprecate all version of `@semantic-release/error` < 2.0.1 as it contains a bug that prevent the name and stacktrace to be included. See https://github.com/semantic-release/error/pull/71.

See https://github.com/semantic-release/semantic-release/pull/480#pullrequestreview-72660167

cc /@felixfbecker

codecov[bot]: # [Codecov](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=h1) Report
> Merging [#491](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=desc) into [caribou](https://codecov.io/gh/semantic-release/semantic-release/commit/8a1dd7b324149e900675c619197daf8b1e643ec7?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/semantic-release/semantic-release/pull/491/graphs/tree.svg?token=hENTgPdVFG&src=pr&width=650&height=150)](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree)

```diff
@@          Coverage Diff           @@
##           caribou   #491   +/-   ##
======================================
  Coverage      100%   100%           
======================================
  Files           15     15           
  Lines          278    277    -1     
  Branches        43     41    -2     
======================================
- Hits           278    277    -1
```


| [Impacted Files](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/lib/logger.js](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree#diff-c3JjL2xpYi9sb2dnZXIuanM=) | `100% <100%> (ø)` | :arrow_up: |
| [src/cli.js](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree#diff-c3JjL2NsaS5qcw==) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=footer). Last update [8a1dd7b...c2bc8fa](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

codecov[bot]: # [Codecov](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=h1) Report
> Merging [#491](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=desc) into [caribou](https://codecov.io/gh/semantic-release/semantic-release/commit/8a1dd7b324149e900675c619197daf8b1e643ec7?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/semantic-release/semantic-release/pull/491/graphs/tree.svg?src=pr&width=650&token=hENTgPdVFG&height=150)](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree)

```diff
@@          Coverage Diff           @@
##           caribou   #491   +/-   ##
======================================
  Coverage      100%   100%           
======================================
  Files           15     15           
  Lines          278    277    -1     
  Branches        43     41    -2     
======================================
- Hits           278    277    -1
```


| [Impacted Files](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/cli.js](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree#diff-c3JjL2NsaS5qcw==) | `100% <100%> (ø)` | :arrow_up: |
| [src/lib/logger.js](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree#diff-c3JjL2xpYi9sb2dnZXIuanM=) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=footer). Last update [8a1dd7b...c2bc8fa](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

codecov[bot]: # [Codecov](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=h1) Report
> Merging [#491](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=desc) into [caribou](https://codecov.io/gh/semantic-release/semantic-release/commit/8a1dd7b324149e900675c619197daf8b1e643ec7?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/semantic-release/semantic-release/pull/491/graphs/tree.svg?src=pr&width=650&token=hENTgPdVFG&height=150)](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree)

```diff
@@          Coverage Diff           @@
##           caribou   #491   +/-   ##
======================================
  Coverage      100%   100%           
======================================
  Files           15     15           
  Lines          278    277    -1     
  Branches        43     41    -2     
======================================
- Hits           278    277    -1
```


| [Impacted Files](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/cli.js](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree#diff-c3JjL2NsaS5qcw==) | `100% <100%> (ø)` | :arrow_up: |
| [src/lib/logger.js](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree#diff-c3JjL2xpYi9sb2dnZXIuanM=) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=footer). Last update [8a1dd7b...899441c](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

codecov[bot]: # [Codecov](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=h1) Report
> Merging [#491](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=desc) into [caribou](https://codecov.io/gh/semantic-release/semantic-release/commit/8a1dd7b324149e900675c619197daf8b1e643ec7?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/semantic-release/semantic-release/pull/491/graphs/tree.svg?src=pr&width=650&token=hENTgPdVFG&height=150)](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree)

```diff
@@          Coverage Diff           @@
##           caribou   #491   +/-   ##
======================================
  Coverage      100%   100%           
======================================
  Files           15     15           
  Lines          278    280    +2     
  Branches        43     46    +3     
======================================
+ Hits           278    280    +2
```


| [Impacted Files](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/cli.js](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree#diff-c3JjL2NsaS5qcw==) | `100% <100%> (ø)` | :arrow_up: |
| [src/lib/logger.js](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree#diff-c3JjL2xpYi9sb2dnZXIuanM=) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=footer). Last update [8a1dd7b...f1621c2](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

pvdlg: I think every feedback has been addressed now.
@felixfbecker @gr2m could you review ?
codecov[bot]: # [Codecov](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=h1) Report
> Merging [#491](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=desc) into [caribou](https://codecov.io/gh/semantic-release/semantic-release/commit/8a1dd7b324149e900675c619197daf8b1e643ec7?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/semantic-release/semantic-release/pull/491/graphs/tree.svg?height=150&width=650&token=hENTgPdVFG&src=pr)](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree)

```diff
@@          Coverage Diff           @@
##           caribou   #491   +/-   ##
======================================
  Coverage      100%   100%           
======================================
  Files           15     15           
  Lines          278    280    +2     
  Branches        43     46    +3     
======================================
+ Hits           278    280    +2
```


| [Impacted Files](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/cli.js](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree#diff-c3JjL2NsaS5qcw==) | `100% <100%> (ø)` | :arrow_up: |
| [src/lib/logger.js](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=tree#diff-c3JjL2xpYi9sb2dnZXIuanM=) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=footer). Last update [8a1dd7b...d0a6fbe](https://codecov.io/gh/semantic-release/semantic-release/pull/491?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

gr2m: I'd squash the commits to just `fix: log error properties and check SemanticReleaseError by .semanticRelease property`, the current commit message is no longer correct, right?
pvdlg: > I'd squash the commits to just fix: log error properties and check SemanticReleaseError by .semanticRelease property, the current commit message is no longer correct, right?

I renamed the commit properly. any reason to squash the 3 commits in one? They fix different things. So as much as possible I rather to smaller, specific commit. That create a release note with more info and make the repo history clearer.
felixfbecker: Agree, as long as every commit works on it's own (a build on this would succeed) I wouldn't squash them
pvdlg: > Agree, as long as every commit works on it's own (a build on this would succeed) I wouldn't squash them

Yes I make sure of that when I create multiple commits in a PR. If for some reasons I fix two related thing but can't make 2 commits that work by themselves, then I create 1 commit and I mention both fix in the body.

Make me think it would be nice to include commit body in release note (maybe in segment with ▶ that would allow to unfold/expand the body). But that's another story.
gr2m: > I renamed the commit properly

Even better, I just wanted to safe you the extra work and make sure we don't merge it with the old commit message to avoid misleading change logs :)

Patch:
diff --git a/package.json b/package.json
index a1d5b61..7f8247a 100644
--- a/package.json
+++ b/package.json
@@ -17,7 +17,7 @@
   "dependencies": {
     "@semantic-release/commit-analyzer": "^3.0.1",
     "@semantic-release/condition-travis": "^6.0.0",
-    "@semantic-release/error": "^2.0.0",
+    "@semantic-release/error": "^2.1.0",
     "@semantic-release/last-release-npm": "^2.0.0",
     "@semantic-release/release-notes-generator": "^5.0.0",
     "chalk": "^2.3.0",
diff --git a/src/cli.js b/src/cli.js
index 7e68486..cf969b2 100755
--- a/src/cli.js
+++ b/src/cli.js
@@ -1,5 +1,4 @@
 const program = require('commander');
-const SemanticReleaseError = require('@semantic-release/error');
 const logger = require('./lib/logger');
 
 function list(values) {
@@ -49,11 +48,11 @@ module.exports = async () => {
   } catch (err) {
     // If error is a SemanticReleaseError then it's an expected exception case (no release to be done, running on a PR etc..) and the cli will return with 0
     // Otherwise it's an unexpected error (configuration issue, code issue, plugin issue etc...) and the cli will return 1
-    if (err instanceof SemanticReleaseError) {
+    if (err.semanticRelease) {
       logger.log(`%s ${err.message}`, err.code);
     } else {
       process.exitCode = 1;
-      logger.error(err);
+      logger.error('An error occurred while running semantic-release: %O', err);
     }
   }
 };
diff --git a/src/lib/logger.js b/src/lib/logger.js
index 9116e72..6990398 100644
--- a/src/lib/logger.js
+++ b/src/lib/logger.js
@@ -6,13 +6,18 @@ const chalk = require('chalk');
 module.exports = {
   log(...args) {
     const [format, ...rest] = args;
-    console.log(`${chalk.grey('[Semantic release]:')} ${format}`, ...rest.map(arg => chalk.magenta(arg)));
+    console.log(
+      `${chalk.grey('[Semantic release]:')}${typeof format === 'string'
+        ? ` ${format.replace(/%[^%]/g, seq => chalk.magenta(seq))}`
+        : ''}`,
+      ...(typeof format === 'string' ? [] : [format]).concat(rest)
+    );
   },
   error(...args) {
     const [format, ...rest] = args;
     console.error(
-      `${chalk.grey('[Semantic release]:')} ${chalk.red(format instanceof Error ? format.stack : format)}`,
-      ...rest.map(arg => chalk.red(arg instanceof Error ? arg.stack : arg))
+      `${chalk.grey('[Semantic release]:')}${typeof format === 'string' ? ` ${chalk.red(format)}` : ''}`,
+      ...(typeof format === 'string' ? [] : [format]).concat(rest)
     );
   },
 };


Test patch:
diff --git a/test/fixtures/plugin-error-a.js b/test/fixtures/plugin-error-a.js
index 27c8eaf..2a4fe3d 100644
--- a/test/fixtures/plugin-error-a.js
+++ b/test/fixtures/plugin-error-a.js
@@ -1,3 +1,5 @@
 module.exports = function(config, options, cb) {
-  cb(new Error('a'));
+  const error = new Error('a');
+  error.errorProperty = 'errorProperty';
+  cb(error);
 };
diff --git a/test/fixtures/plugin-error-inherited.js b/test/fixtures/plugin-error-inherited.js
new file mode 100644
index 0000000..12aad0f
--- /dev/null
+++ b/test/fixtures/plugin-error-inherited.js
@@ -0,0 +1,14 @@
+const SemanticReleaseError = require('@semantic-release/error');
+
+class InheritedError extends SemanticReleaseError {
+  constructor(message, code, newProperty) {
+    super(message);
+    Error.captureStackTrace(this, this.constructor);
+    this.name = this.constructor.name;
+    this.code = code;
+  }
+}
+
+module.exports = function(config, options, cb) {
+  cb(new InheritedError('Inherited error', 'EINHERITED'));
+};
diff --git a/test/integration.test.js b/test/integration.test.js
index a6496f2..d541497 100644
--- a/test/integration.test.js
+++ b/test/integration.test.js
@@ -17,6 +17,7 @@ const env = {
 const cli = require.resolve('../bin/semantic-release');
 const noop = require.resolve('../src/lib/plugin-noop');
 const pluginError = require.resolve('./fixtures/plugin-error-a');
+const pluginInheritedError = require.resolve('./fixtures/plugin-error-inherited');
 
 test.before(async t => {
   await mockServer.start();
@@ -35,7 +36,6 @@ test.beforeEach(async t => {
 });
 
 test.afterEach.always(async t => {
-  console.log();
   // Restore process.env
   process.env = Object.assign({}, t.context.env);
   // Restore the current working directory
@@ -574,7 +574,7 @@ test.serial('Run via JS API', async t => {
   await semanticRelease({githubToken, verifyConditions: [noop, noop], debug: true});
 
   t.true(t.context.log.calledWithMatch(/Published Github release: /, new RegExp(`release-url/${version}`)));
-  t.true(t.context.log.calledWithMatch(/Publishing version %s to npm registry %s/, version, registry.uri));
+  t.true(t.context.log.calledWithMatch(/Publishing version .* to npm registry/, version, registry.uri));
 
   // Verify package.json and has been updated
   t.is((await readJson('./package.json')).version, version);
@@ -615,7 +615,7 @@ test.serial('Returns and error code if NPM token is invalid', async t => {
   t.is(code, 1);
 });
 
-test.serial('Log unexpected errors from plugins', async t => {
+test.serial('Log unexpected errors from plugins and exit with 1', async t => {
   const packageName = 'test-module-9';
   const repo = 'test-repo';
   // Create a git repository, set the current working directory at the root of the repo
@@ -634,11 +634,39 @@ test.serial('Log unexpected errors from plugins', async t => {
   await gitCommits(['feat: Initial commit']);
   t.log('$ semantic-release');
   let {stderr, code} = await execa(cli, [], {env, reject: false});
+  // Verify the type and message are logged
   t.regex(stderr, /Error: a/);
+  // Verify the the stacktrace is logged
   t.regex(stderr, new RegExp(pluginError));
+  // Verify the Error properties are logged
+  t.regex(stderr, /errorProperty: 'errorProperty'/);
   t.is(code, 1);
 });
 
+test.serial('Log errors inheriting SemanticReleaseError and exit with 0', async t => {
+  const packageName = 'test-module-10';
+  const repo = 'test-repo';
+  // Create a git repository, set the current working directory at the root of the repo
+  t.log('Create git repository and package.json');
+  await gitRepo();
+  // Create package.json in repository root
+  await writeJson('./package.json', {
+    name: packageName,
+    version: '0.0.0-dev',
+    repository: {url: `git+https://github.com/${repo}/${packageName}`},
+    release: {githubUrl: mockServer.url, verifyConditions: pluginInheritedError},
+  });
+
+  /** Initial release **/
+  t.log('Commit a feature');
+  await gitCommits(['feat: Initial commit']);
+  t.log('$ semantic-release');
+  let {stdout, code} = await execa(cli, [], {env, reject: false});
+  // Verify the type and message are logged
+  t.regex(stdout, /EINHERITED Inherited error/);
+  t.is(code, 0);
+});
+
 test.serial('CLI returns error code and prints help if called with a command', async t => {
   t.log('$ semantic-release pre');
   let {stdout, code} = await execa(cli, ['pre'], {env, reject: false});
diff --git a/test/logger.test.js b/test/logger.test.js
index 9b6fbfb..3ea564c 100644
--- a/test/logger.test.js
+++ b/test/logger.test.js
@@ -1,5 +1,5 @@
 import test from 'ava';
-import {stub} from 'sinon';
+import {stub, match} from 'sinon';
 import logger from '../src/lib/logger';
 
 test.beforeEach(t => {
@@ -20,18 +20,29 @@ test.serial('Basic log', t => {
   t.true(t.context.error.calledWithMatch(/.*test error/));
 });
 
+test.serial('Log object', t => {
+  const obj = {a: 1, b: '2'};
+  logger.log(obj);
+  logger.error(obj);
+
+  t.true(t.context.log.calledWithMatch(match.string, obj));
+  t.true(t.context.error.calledWithMatch(match.string, obj));
+});
+
 test.serial('Log with string formatting', t => {
   logger.log('test log %s', 'log value');
   logger.error('test error %s', 'error value');
 
-  t.true(t.context.log.calledWithMatch(/.*test log %s/, 'log value'));
-  t.true(t.context.error.calledWithMatch(/.*test error %s/, 'error value'));
+  t.true(t.context.log.calledWithMatch(/.*test log/, 'log value'));
+  t.true(t.context.error.calledWithMatch(/.*test error/, 'error value'));
 });
 
-test.serial('Log with error stacktrace', t => {
-  logger.error(new Error('error message'));
-  logger.error('test error %s', new Error('other error message'));
+test.serial('Log with error stacktrace and properties', t => {
+  const error = new Error('error message');
+  logger.error(error);
+  const otherError = new Error('other error message');
+  logger.error('test error %O', otherError);
 
-  t.true(t.context.error.calledWithMatch(/.*test error %s/, /Error: other error message(\s|.)*?logger\.test\.js/));
-  t.true(t.context.error.calledWithMatch(/Error: error message(\s|.)*?logger\.test\.js/));
+  t.true(t.context.error.calledWithMatch(match.string, error));
+  t.true(t.context.error.calledWithMatch(/.*test error/, otherError));
 });
diff --git a/test/verify-pkg.test.js b/test/verify-pkg.test.js
index f9ad6d2..06c70b6 100644
--- a/test/verify-pkg.test.js
+++ b/test/verify-pkg.test.js
@@ -2,12 +2,12 @@ import test from 'ava';
 import SemanticReleaseError from '@semantic-release/error';
 import verify from '../src/lib/verify-pkg';
 
-test.only('Verify name and repository', t => {
+test('Verify name and repository', t => {
   // Call the verify module with package
   t.notThrows(() => verify({name: 'package', repository: {url: 'http://github.com/whats/up.git'}}));
 });
 
-test.only('Return error for missing package name', t => {
+test('Return error for missing package name', t => {
   // Call the verify module with package
   const error = t.throws(() => verify({repository: {url: 'http://github.com/whats/up.git'}}));
   // Verify error code and type
@@ -15,7 +15,7 @@ test.only('Return error for missing package name', t => {
   t.true(error instanceof SemanticReleaseError);
 });
 
-test.only('Return error for missing repository', t => {
+test('Return error for missing repository', t => {
   // Call the verify module with package
   const error = t.throws(() => verify({name: 'package'}));
   // Verify error code and type
@@ -23,7 +23,7 @@ test.only('Return error for missing repository', t => {
   t.true(error instanceof SemanticReleaseError);
 });
 
-test.only('Return error for missing repository url', t => {
+test('Return error for missing repository url', t => {
   // Call the verify module with package
   const error = t.throws(() => verify({name: 'package', repository: {}}));
   // Verify error code and type