From 08f6372c0b1cb9ae77ec5da98cff6c5dd6dcde79 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Mon, 24 Oct 2016 13:09:34 -0700 Subject: [PATCH] errors: add internal/errors.js Add the internal/errors.js core mechanism. --- doc/guides/using-internal-errors.md | 141 ++++++++++++++++++++++++++ lib/internal/errors.js | 88 ++++++++++++++++ node.gyp | 1 + test/common.js | 15 +++ test/parallel/test-internal-errors.js | 116 +++++++++++++++++++++ 5 files changed, 361 insertions(+) create mode 100644 doc/guides/using-internal-errors.md create mode 100644 lib/internal/errors.js create mode 100644 test/parallel/test-internal-errors.js diff --git a/doc/guides/using-internal-errors.md b/doc/guides/using-internal-errors.md new file mode 100644 index 00000000000000..9f8634dc233b9e --- /dev/null +++ b/doc/guides/using-internal-errors.md @@ -0,0 +1,141 @@ +# Using the internal/errors.js Module + +## What is internal/errors.js + +The `require('internal/errors')` module is an internal-only module that can be +used to produce `Error`, `TypeError` and `RangeError` instances that use a +static, permanent error code and an optionally parameterized message. + +The intent of the module is to allow errors provided by Node.js to be assigned a +permanent identifier. Without a permanent identifier, userland code may need to +inspect error messages to distinguish one error from another. An unfortunate +result of that practice is that changes to error messages result in broken code +in the ecosystem. For that reason, Node.js has considered error message changes +to be breaking changes. By providing a permanent identifier for a specific +error, we reduce the need for userland code to inspect error messages. + +*Note*: Switching an existing error to use the `internal/errors` module must be +considered a `semver-major` change. However, once using `internal/errors`, +changes to `internal/errors` error messages will be handled as `semver-minor` +or `semver-patch`. + +## Using internal/errors.js + +The `internal/errors` module exposes three custom `Error` classes that +are intended to replace existing `Error` objects within the Node.js source. + +For instance, an existing `Error` such as: + +```js + var err = new TypeError('Expected string received ' + type); +``` + +Can be replaced by first adding a new error key into the `internal/errors.js` +file: + +```js +E('FOO', 'Expected string received %s'); +``` + +Then replacing the existing `new TypeError` in the code: + +```js + const errors = require('internal/errors'); + // ... + var err = new errors.TypeError('FOO', type); +``` + +## Adding new errors + +New static error codes are added by modifying the `internal/errors.js` file +and appending the new error codes to the end using the utility `E()` method. + +```js +E('EXAMPLE_KEY1', 'This is the error value'); +E('EXAMPLE_KEY2', (a, b) => return `${a} ${b}`); +``` + +The first argument passed to `E()` is the static identifier. The second +argument is either a String with optional `util.format()` style replacement +tags (e.g. `%s`, `%d`), or a function returning a String. The optional +additional arguments passed to the `errors.message()` function (which is +used by the `errors.Error`, `errors.TypeError` and `errors.RangeError` classes), +will be used to format the error message. + +## Documenting new errors + +Whenever a new static error code is added and used, corresponding documentation +for the error code should be added to the `doc/api/errors.md` file. This will +give users a place to go to easily look up the meaning of individual error +codes. + + +## API + +### Class: errors.Error(key[, args...]) + +* `key` {String} The static error identifier +* `args...` {Any} Zero or more optional arguments + +```js +const errors = require('internal/errors'); + +var arg1 = 'foo'; +var arg2 = 'bar'; +const myError = new errors.Error('KEY', arg1, arg2); +throw myError; +``` + +The specific error message for the `myError` instance will depend on the +associated value of `KEY` (see "Adding new errors"). + +The `myError` object will have a `code` property equal to the `key` and a +`name` property equal to `Error[${key}]`. + +### Class: errors.TypeError(key[, args...]) + +* `key` {String} The static error identifier +* `args...` {Any} Zero or more optional arguments + +```js +const errors = require('internal/errors'); + +var arg1 = 'foo'; +var arg2 = 'bar'; +const myError = new errors.TypeError('KEY', arg1, arg2); +throw myError; +``` + +The specific error message for the `myError` instance will depend on the +associated value of `KEY` (see "Adding new errors"). + +The `myError` object will have a `code` property equal to the `key` and a +`name` property equal to `TypeError[${key}]`. + +### Class: errors.RangeError(key[, args...]) + +* `key` {String} The static error identifier +* `args...` {Any} Zero or more optional arguments + +```js +const errors = require('internal/errors'); + +var arg1 = 'foo'; +var arg2 = 'bar'; +const myError = new errors.RangeError('KEY', arg1, arg2); +throw myError; +``` + +The specific error message for the `myError` instance will depend on the +associated value of `KEY` (see "Adding new errors"). + +The `myError` object will have a `code` property equal to the `key` and a +`name` property equal to `RangeError[${key}]`. + +### Method: errors.message(key, args) + +* `key` {String} The static error identifier +* `args` {Array} Zero or more optional arguments passed as an Array +* Returns: {String} + +Returns the formatted error message string for the given `key`. diff --git a/lib/internal/errors.js b/lib/internal/errors.js new file mode 100644 index 00000000000000..f2376f70371c60 --- /dev/null +++ b/lib/internal/errors.js @@ -0,0 +1,88 @@ +'use strict'; + +// The whole point behind this internal module is to allow Node.js to no +// longer be forced to treat every error message change as a semver-major +// change. The NodeError classes here all expose a `code` property whose +// value statically and permanently identifies the error. While the error +// message may change, the code should not. + +const kCode = Symbol('code'); +const messages = new Map(); + +var assert, util; +function lazyAssert() { + if (!assert) + assert = require('assert'); + return assert; +} + +function lazyUtil() { + if (!util) + util = require('util'); + return util; +} + +function makeNodeError(Base) { + return class NodeError extends Base { + constructor(key, ...args) { + super(message(key, args)); + this[kCode] = key; + Error.captureStackTrace(this, NodeError); + } + + get name() { + return `${super.name}[${this[kCode]}]`; + } + + get code() { + return this[kCode]; + } + }; +} + +function message(key, args) { + const assert = lazyAssert(); + assert.strictEqual(typeof key, 'string'); + const util = lazyUtil(); + const msg = messages.get(key); + assert(msg, `An invalid error message key was used: ${key}.`); + let fmt = util.format; + if (typeof msg === 'function') { + fmt = msg; + } else { + if (args === undefined || args.length === 0) + return msg; + args.unshift(msg); + } + return String(fmt.apply(null, args)); +} + +// Utility function for registering the error codes. Only used here. Exported +// *only* to allow for testing. +function E(sym, val) { + messages.set(sym, typeof val === 'function' ? val : String(val)); +} + +module.exports = exports = { + message, + Error: makeNodeError(Error), + TypeError: makeNodeError(TypeError), + RangeError: makeNodeError(RangeError), + E // This is exported only to facilitate testing. +}; + +// To declare an error message, use the E(sym, val) function above. The sym +// must be an upper case string. The val can be either a function or a string. +// The return value of the function must be a string. +// Examples: +// E('EXAMPLE_KEY1', 'This is the error value'); +// E('EXAMPLE_KEY2', (a, b) => return `${a} ${b}`); +// +// Once an error code has been assigned, the code itself MUST NOT change and +// any given error code must never be reused to identify a different error. +// +// Any error code added here should also be added to the documentation +// +// Note: Please try to keep these in alphabetical order +E('ERR_ASSERTION', (msg) => msg); +// Add new errors from here... diff --git a/node.gyp b/node.gyp index 6cccf74a89f771..b70ae25e0653ae 100644 --- a/node.gyp +++ b/node.gyp @@ -82,6 +82,7 @@ 'lib/internal/cluster/shared_handle.js', 'lib/internal/cluster/utils.js', 'lib/internal/cluster/worker.js', + 'lib/internal/errors.js', 'lib/internal/freelist.js', 'lib/internal/fs.js', 'lib/internal/linkedlist.js', diff --git a/test/common.js b/test/common.js index 1d7674a2ff7a00..5f7dc253736e8d 100644 --- a/test/common.js +++ b/test/common.js @@ -616,3 +616,18 @@ exports.WPT = { assert.fail(undefined, undefined, `Reached unreachable code: ${desc}`); } }; + +// Useful for testing expected internal/error objects +exports.expectsError = function expectsError(code, type, message) { + return function(error) { + assert.strictEqual(error.code, code); + if (type !== undefined) + assert(error instanceof type, 'error is not the expected type'); + if (message !== undefined) { + if (!util.isRegExp(message)) + message = new RegExp(String(message)); + assert(message.test(error.message), 'error.message does not match'); + } + return true; + }; +}; diff --git a/test/parallel/test-internal-errors.js b/test/parallel/test-internal-errors.js new file mode 100644 index 00000000000000..6844ce539ee004 --- /dev/null +++ b/test/parallel/test-internal-errors.js @@ -0,0 +1,116 @@ +// Flags: --expose-internals +'use strict'; + +const common = require('../common'); +const errors = require('internal/errors'); +const assert = require('assert'); + +errors.E('TEST_ERROR_1', 'Error for testing purposes: %s'); +errors.E('TEST_ERROR_2', (a, b) => `${a} ${b}`); + +const err1 = new errors.Error('TEST_ERROR_1', 'test'); +const err2 = new errors.TypeError('TEST_ERROR_1', 'test'); +const err3 = new errors.RangeError('TEST_ERROR_1', 'test'); +const err4 = new errors.Error('TEST_ERROR_2', 'abc', 'xyz'); + +assert(err1 instanceof Error); +assert.strictEqual(err1.name, 'Error[TEST_ERROR_1]'); +assert.strictEqual(err1.message, 'Error for testing purposes: test'); +assert.strictEqual(err1.code, 'TEST_ERROR_1'); + +assert(err2 instanceof TypeError); +assert.strictEqual(err2.name, 'TypeError[TEST_ERROR_1]'); +assert.strictEqual(err2.message, 'Error for testing purposes: test'); +assert.strictEqual(err2.code, 'TEST_ERROR_1'); + +assert(err3 instanceof RangeError); +assert.strictEqual(err3.name, 'RangeError[TEST_ERROR_1]'); +assert.strictEqual(err3.message, 'Error for testing purposes: test'); +assert.strictEqual(err3.code, 'TEST_ERROR_1'); + +assert(err4 instanceof Error); +assert.strictEqual(err4.name, 'Error[TEST_ERROR_2]'); +assert.strictEqual(err4.message, 'abc xyz'); +assert.strictEqual(err4.code, 'TEST_ERROR_2'); + +assert.throws( + () => new errors.Error('TEST_FOO_KEY'), + /^AssertionError: An invalid error message key was used: TEST_FOO_KEY.$/); +// Calling it twice yields same result (using the key does not create it) +assert.throws( + () => new errors.Error('TEST_FOO_KEY'), + /^AssertionError: An invalid error message key was used: TEST_FOO_KEY.$/); +assert.throws( + () => new errors.Error(1), + /^AssertionError: 'number' === 'string'$/); +assert.throws( + () => new errors.Error({}), + /^AssertionError: 'object' === 'string'$/); +assert.throws( + () => new errors.Error([]), + /^AssertionError: 'object' === 'string'$/); +assert.throws( + () => new errors.Error(true), + /^AssertionError: 'boolean' === 'string'$/); +assert.throws( + () => new errors.TypeError(1), + /^AssertionError: 'number' === 'string'$/); +assert.throws( + () => new errors.TypeError({}), + /^AssertionError: 'object' === 'string'$/); +assert.throws( + () => new errors.TypeError([]), + /^AssertionError: 'object' === 'string'$/); +assert.throws( + () => new errors.TypeError(true), + /^AssertionError: 'boolean' === 'string'$/); +assert.throws( + () => new errors.RangeError(1), + /^AssertionError: 'number' === 'string'$/); +assert.throws( + () => new errors.RangeError({}), + /^AssertionError: 'object' === 'string'$/); +assert.throws( + () => new errors.RangeError([]), + /^AssertionError: 'object' === 'string'$/); +assert.throws( + () => new errors.RangeError(true), + /^AssertionError: 'boolean' === 'string'$/); + + +// Tests for common.expectsError +assert.doesNotThrow(() => { + assert.throws(() => { + throw new errors.TypeError('TEST_ERROR_1', 'a'); + }, common.expectsError('TEST_ERROR_1')); +}); + +assert.doesNotThrow(() => { + assert.throws(() => { + throw new errors.TypeError('TEST_ERROR_1', 'a'); + }, common.expectsError('TEST_ERROR_1', TypeError, /^Error for testing/)); +}); + +assert.doesNotThrow(() => { + assert.throws(() => { + throw new errors.TypeError('TEST_ERROR_1', 'a'); + }, common.expectsError('TEST_ERROR_1', TypeError)); +}); + +assert.doesNotThrow(() => { + assert.throws(() => { + throw new errors.TypeError('TEST_ERROR_1', 'a'); + }, common.expectsError('TEST_ERROR_1', Error)); +}); + +assert.throws(() => { + assert.throws(() => { + throw new errors.TypeError('TEST_ERROR_1', 'a'); + }, common.expectsError('TEST_ERROR_1', RangeError)); +}, /^AssertionError: error is not the expected type/); + +assert.throws(() => { + assert.throws(() => { + throw new errors.TypeError('TEST_ERROR_1', 'a'); + }, common.expectsError('TEST_ERROR_1', TypeError, /^Error for testing 2/)); +}, /^AssertionError: error.message does not match/);