feat: support upcasting into VError

lib/verror.js

@@ -151,6 +151,83 @@ function parseConstructorArguments(args) {
     };
 }
 
+/**
+ * Helper function to set parsed arguments from the VError constructor and set
+ * them on the target error. This has been extracted from the constructor itself
+ * so that it can be called independently from an `upcast` method. This allows
+ * us to use a mix-in pattern to augment standard Errors with VError semantics.
+ * @private
+ * @param {Error} targetErr - if called from the VError constructor, a VError.
+ * if called from the VError.upcast method, then a standard Error.
+ * @param {String} shortmessage - result of sprintf(sprintf_args), taking
+ *              `options.strict` into account as described in README.md.
+ * @param {ParsedOptions} parsed - parsed options object from the
+ * parseConstructorArguments method
+ * @param {Boolean} isCtor - when true, this is called via the constructor
+ * @returns {undefined} parsed options
+ */
+// eslint-disable-next-line handle-callback-err
+function setVErrorFields(targetErr, parsed, isCtor) {
+    let ctor, message, k;
+
+    // this function can be used to upcast a standard Error into a VError.
+    // be defensive here and don't stomp over fields blindly.
+
+    /*
+     * If we've been given a name, apply it now.
+     */
+    if (isCtor && parsed.options.name) {
+        assert.string(parsed.options.name, 'error\'s "name" must be a string');
+        targetErr.name = parsed.options.name;
+    }
+
+    /*
+     * For debugging, we keep track of the original short message (attached
+     * this Error particularly) separately from the complete message (which
+     * includes the messages of our cause chain).
+     */
+    targetErr.jse_shortmsg = parsed.shortmessage;
+    message = parsed.shortmessage;
+
+    /*
+     * If we've been given a cause, record a reference to it and update our
+     * message appropriately.
+     */
+    const cause = parsed.options.cause;
+    if (cause) {
+        VError._assertError(cause, '"cause" must be an Error');
+        targetErr.jse_cause = cause;
+
+        if (!parsed.options.skipCauseMessage) {
+            message += ': ' + cause.message;
+        }
+    }
+
+    /*
+     * If we've been given an object with properties, shallow-copy that
+     * here.  We don't want to use a deep copy in case there are non-plain
+     * objects here, but we don't want to use the original object in case
+     * the caller modifies it later.
+     */
+    targetErr.jse_info = {};
+    if (parsed.options.info) {
+        // eslint-disable-next-line guard-for-in
+        for (k in parsed.options.info) {
+            targetErr.jse_info[k] = parsed.options.info[k];
+        }
+    }
+
+    if (isCtor) {
+        targetErr.message = message;
+        Error.call(targetErr, message);
+    }
+
+    if (isCtor && Error.captureStackTrace) {
+        ctor = parsed.options.constructorOpt || targetErr.constructor;
+        Error.captureStackTrace(targetErr, ctor);
+    }
+}
+
 /**
  * @public
  * @typedef {Object} VErrorOptions Options
@@ -203,15 +280,13 @@ function parseConstructorArguments(args) {
  * new VError(sprintf_args...)
  */
 function VError(...args) {
-    let obj, ctor, message, k;
-
     /*
      * This is a regrettable pattern, but JavaScript's built-in Error class
      * is defined to work this way, so we allow the constructor to be called
      * without "new".
      */
     if (!(this instanceof VError)) {
-        obj = Object.create(VError.prototype);
+        const obj = Object.create(VError.prototype);
         VError.apply(obj, arguments);
         return obj;
     }
@@ -225,57 +300,7 @@ function VError(...args) {
         strict: false
     });
 
-    /*
-     * If we've been given a name, apply it now.
-     */
-    if (parsed.options.name) {
-        assert.string(parsed.options.name, 'error\'s "name" must be a string');
-        this.name = parsed.options.name;
-    }
-
-    /*
-     * For debugging, we keep track of the original short message (attached
-     * this Error particularly) separately from the complete message (which
-     * includes the messages of our cause chain).
-     */
-    this.jse_shortmsg = parsed.shortmessage;
-    message = parsed.shortmessage;
-
-    /*
-     * If we've been given a cause, record a reference to it and update our
-     * message appropriately.
-     */
-    const cause = parsed.options.cause;
-    if (cause) {
-        VError._assertError(cause, '"cause" must be an Error');
-        this.jse_cause = cause;
-
-        if (!parsed.options.skipCauseMessage) {
-            message += ': ' + cause.message;
-        }
-    }
-
-    /*
-     * If we've been given an object with properties, shallow-copy that
-     * here.  We don't want to use a deep copy in case there are non-plain
-     * objects here, but we don't want to use the original object in case
-     * the caller modifies it later.
-     */
-    this.jse_info = {};
-    if (parsed.options.info) {
-        // eslint-disable-next-line guard-for-in
-        for (k in parsed.options.info) {
-            this.jse_info[k] = parsed.options.info[k];
-        }
-    }
-
-    this.message = message;
-    Error.call(this, message);
-
-    if (Error.captureStackTrace) {
-        ctor = parsed.options.constructorOpt || this.constructor;
-        Error.captureStackTrace(this, ctor);
-    }
+    setVErrorFields(this, parsed, true);
 
     return this;
 }
@@ -370,12 +395,48 @@ VError._assertError = function _assertError(err, msg) {
  * @param {Error} err - error
  * @return {Boolean} is a VError or VError sub-class
  */
-VError.isVError = function assignInfo(err) {
+VError.isVError = function isVError(err) {
     // We are checking on internals here instead of using
     // `err instanceof VError` to being compatible with the original VError lib.
     return err && err.hasOwnProperty('jse_info');
 };
 
+/**
+ * Upcasts a standard error into a VError.
+ *
+ * @public
+ * @static
+ * @memberof VError
+ * @param {Error} err - error
+ * @return {VError} return the original Error, now upcasted into VError
+ */
+VError.upcast = function upcast(err, ...args) {
+    // if this is already a VError, return.
+    if (VError.isVError(err)) {
+        return err;
+    }
+
+    // super poor man's upcasting. when we upcast, we don't want to create a new
+    // error as it'll be a new object and can make core file analysis more
+    // difficult. we want to retain the original error and original properties
+    // while extending it with VError semantics.
+    //
+    // what we want is a bit more old school, like a mix-in. parse any
+    // arguments that have come in, then set VError specific fields as needed.
+    // we don't need/want to muck with the prototype as that could be more
+    // expensive and/or causing deoptimization.
+    const parsedArgs = parseConstructorArguments({
+        argv: args,
+        strict: false
+    });
+    setVErrorFields(err, parsedArgs, false);
+
+    // finally, extend the err instance with VError methods.
+    Object.assign(err, VError.prototype);
+
+    return err;
+};
+
 /**
  * Appends any keys/fields to the `jse_info`. This can stomp over any existing
  * fields.

test/info.test.js

@@ -206,15 +206,5 @@ describe('info', function() {
                 remote_ip: '127.0.0.1'
             });
         });
-
-        it('support not break when err is not a VError', function() {
-            const err = new Error('bad');
-
-            assert.throws(function() {
-                VError.assignInfo(err, {
-                    remote_ip: '127.0.0.1'
-                });
-            }, /err must be an instance of VError/);
-        });
     });
 });

test/upcast.test.js

@@ -0,0 +1,32 @@
+'use strict';
+
+/*
+ * tests the ability to upcast a standard Error into a VError without wrapping
+ * it.
+ */
+
+const assert = require('assert');
+
+const { VError } = require('../lib/verror');
+
+describe('info', function() {
+    it('should upcast Error into VError', function() {
+        const err = new TypeError('bad');
+        const upcasted = VError.upcast(err);
+
+        // should mutate in place
+        assert.equal(upcasted, err);
+        assert.ok(err.stack);
+        assert.ok(err.name, 'TypeError');
+
+        // should have VError instance methods
+        err.assignInfo({
+            remote_ip: '127.0.0.1'
+        });
+        const info = err.info();
+
+        assert.deepStrictEqual(info, {
+            remote_ip: '127.0.0.1'
+        });
+    });
+});