diff --git a/lib/verror.js b/lib/verror.js index b6575a9..9edb97f 100644 --- a/lib/verror.js +++ b/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. diff --git a/test/info.test.js b/test/info.test.js index fb5faa1..e85fd05 100644 --- a/test/info.test.js +++ b/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/); - }); }); }); diff --git a/test/upcast.test.js b/test/upcast.test.js new file mode 100644 index 0000000..1a40515 --- /dev/null +++ b/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' + }); + }); +});