You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by ra...@apache.org on 2020/03/14 14:37:27 UTC

[cordova-common] branch master updated: feat(CordovaError): support for error cause & more (#121)

This is an automated email from the ASF dual-hosted git repository.

raphinesse pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/cordova-common.git


The following commit(s) were added to refs/heads/master by this push:
     new fe6a73f  feat(CordovaError): support for error cause & more (#121)
fe6a73f is described below

commit fe6a73fc80ebf974b4c27251091de89e1df2c0f3
Author: Raphael von der GrĂ¼n <ra...@gmail.com>
AuthorDate: Sat Mar 14 15:37:17 2020 +0100

    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 8e8a31c..cd6b94d 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 21b5acd..e08e5aa 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 1228f33..2638642 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 8aeda07..0b1911e 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);
     }
 }
 


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org