From fe6a73fc80ebf974b4c27251091de89e1df2c0f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Raphael=20von=20der=20Gr=C3=BCn?= Date: Sat, 14 Mar 2020 15:37:17 +0100 Subject: [PATCH] feat(CordovaError): support for error cause & more (#121) This commit bases CordovaError on the popular [joyent/node-verror]. We actually use @netflix/nerror, a VError fork, for now. That's because we do not want printf style error formatting support and that fork allows to disable it. There's [ongoing work][1] to integrate that change into the original VError. So basically CordovaError behaves like PError but with all the static methods from VError and different parameter ordering for its constructor. One change that could break some existing tests in repositories that use cordova-common is that `toString` (for errors without a cause argument) now behaves like the Error default again: new CordovaError('foo').toString(); // old result: 'foo' // new result: 'CordovaError: foo' [joyent/node-verror]: https://github.com/joyent/node-verror [1]: https://github.com/joyent/node-verror/issues/63#issuecomment-546067267 --- package-lock.json | 23 +++++++- package.json | 1 + spec/CordovaError/CordovaError.spec.js | 77 +++++++++++++++++++++++--- src/CordovaError.js | 44 ++++++++------- 4 files changed, 117 insertions(+), 28 deletions(-) diff --git a/package-lock.json b/package-lock.json index 8e8a31c6..cd6b94d8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -133,6 +133,16 @@ "eslint-plugin-standard": "^4.0.1" } }, + "@netflix/nerror": { + "version": "1.1.3", + "resolved": "https://registry.npmjs.org/@netflix/nerror/-/nerror-1.1.3.tgz", + "integrity": "sha512-b+MGNyP9/LXkapreJzNUzcvuzZslj/RGgdVVJ16P2wSlYatfLycPObImqVJSmNAdyeShvNeM/pl3sVZsObFueg==", + "requires": { + "assert-plus": "^1.0.0", + "extsprintf": "^1.4.0", + "lodash": "^4.17.15" + } + }, "@nodelib/fs.macchiato": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/@nodelib/fs.macchiato/-/fs.macchiato-1.0.2.tgz", @@ -266,6 +276,11 @@ "es-abstract": "^1.17.0-next.1" } }, + "assert-plus": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/assert-plus/-/assert-plus-1.0.0.tgz", + "integrity": "sha1-8S4PPF13sLHN2RRpQuTpbB5N1SU=" + }, "astral-regex": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/astral-regex/-/astral-regex-1.0.0.tgz", @@ -1061,6 +1076,11 @@ "tmp": "^0.0.33" } }, + "extsprintf": { + "version": "1.4.0", + "resolved": "https://registry.npmjs.org/extsprintf/-/extsprintf-1.4.0.tgz", + "integrity": "sha1-4mifjzVvrWLMplo6kcXfX5VRaS8=" + }, "fast-deep-equal": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-2.0.1.tgz", @@ -1757,8 +1777,7 @@ "lodash": { "version": "4.17.15", "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.15.tgz", - "integrity": "sha512-8xOcRHvCjnocdS5cpwXQXVzmmh5e5+saE2QGoeQmbKmRS6J3VQppPOIt0MnmE+4xlZoumy0GPG0D0MVIQbNA1A==", - "dev": true + "integrity": "sha512-8xOcRHvCjnocdS5cpwXQXVzmmh5e5+saE2QGoeQmbKmRS6J3VQppPOIt0MnmE+4xlZoumy0GPG0D0MVIQbNA1A==" }, "lodash.flattendeep": { "version": "4.4.0", diff --git a/package.json b/package.json index 21b5acdc..e08e5aa0 100644 --- a/package.json +++ b/package.json @@ -23,6 +23,7 @@ "cover": "nyc npm run test:unit" }, "dependencies": { + "@netflix/nerror": "^1.1.3", "ansi": "^0.3.1", "bplist-parser": "^0.2.0", "cross-spawn": "^6.0.5", diff --git a/spec/CordovaError/CordovaError.spec.js b/spec/CordovaError/CordovaError.spec.js index 1228f337..2638642a 100644 --- a/spec/CordovaError/CordovaError.spec.js +++ b/spec/CordovaError/CordovaError.spec.js @@ -17,15 +17,78 @@ under the License. */ -var CordovaError = require('../../src/CordovaError'); +const endent = require('endent'); +const CordovaError = require('../../src/CordovaError'); -describe('CordovaError class', function () { - it('Test 001 : should be constructable', function () { - expect(new CordovaError('error')).toEqual(jasmine.any(CordovaError)); +describe('CordovaError class', () => { + let error; + + beforeEach(() => { + error = new CordovaError('error'); + }); + + it('should be an error', () => { + expect(error).toEqual(jasmine.any(Error)); + }); + + it('should have a name property', () => { + expect(error.name).toEqual('CordovaError'); }); - it('Test 003 : toString works', function () { - var error003_1 = new CordovaError('error'); - expect(error003_1.toString()).toEqual('error'); + it('should have a working toString method', () => { + expect(error.toString()).toEqual('CordovaError: error'); + }); + + describe('given a cause', () => { + let cause; + + beforeEach(() => { + cause = new Error('cause'); + error = new CordovaError('error', cause); + }); + + it('should save it', () => { + expect(error.cause()).toBe(cause); + expect(CordovaError.cause(error)).toBe(cause); + }); + + it('should include the cause in toString result', () => { + const stringifiedError = 'CordovaError: error: cause'; + expect(String(error)).toEqual(stringifiedError); + expect(error.toString()).toEqual(stringifiedError); + }); + + it('should include the cause stack in CordovaError.fullStack', () => { + cause.stack = 'CAUSE_STACK'; + error.stack = 'ERROR_STACK'; + + expect(CordovaError.fullStack(error)).toEqual(endent` + ERROR_STACK + caused by: CAUSE_STACK + `); + }); + }); + + describe('given options', () => { + it('should apply name option', () => { + const name = 'FooError'; + error = new CordovaError('error', { name }); + + expect(error.name).toEqual(name); + }); + + it('should apply cause option', () => { + const cause = new Error('cause'); + error = new CordovaError('error', { cause }); + + expect(CordovaError.cause(error)).toBe(cause); + }); + + it('should apply info option', () => { + const info = { foo: 'bar' }; + error = new CordovaError('error', { info }); + + expect(CordovaError.info(error)).toEqual(info); + }); }); }); diff --git a/src/CordovaError.js b/src/CordovaError.js index 8aeda074..0b1911e9 100644 --- a/src/CordovaError.js +++ b/src/CordovaError.js @@ -17,30 +17,36 @@ under the License. */ +// @ts-check + +const { VError } = require('@netflix/nerror'); + /** - * A derived exception class - * - * Based on: https://stackoverflow.com/a/8460753/380229 + * @public + * @typedef {Object} CordovaErrorOptions + * @param {String} [name] - Name of the error. + * @param {Error} [cause] - Indicates that the new error was caused by `cause`. + * @param {Object} [info] - Specifies arbitrary informational properties. */ -class CordovaError extends Error { - /** - * Creates new CordovaError with given error message - * - * @param {String} message Error message - */ - constructor (message) { - super(message); - Error.captureStackTrace(this, this.constructor); - this.name = this.constructor.name; - } +/** + * A custom exception class derived from VError + */ +class CordovaError extends VError { /** - * Converts this to its string representation - * - * @return {String} Stringified error representation + * @param {String} message - Error message + * @param {Error|CordovaErrorOptions} [causeOrOpts] - The Error that caused + * this to be thrown or a CordovaErrorOptions object. */ - toString () { - return this.message; + constructor (message, causeOrOpts = {}) { + const defaults = { name: 'CordovaError' }; + const overrides = { strict: false, skipPrintf: true }; + const userOpts = causeOrOpts instanceof Error + ? { cause: causeOrOpts } + : causeOrOpts; + const opts = Object.assign(defaults, userOpts, overrides); + + super(opts, message); } }