assert: fix AssertionError, assign error code

Using `assert.AssertionError()` without the `new` keyword results
in a non-intuitive error:

```js
> assert.AssertionError({})
TypeError: Cannot assign to read only property 'name' of function 'function ok(value, message) {
  if (!value) fail(value, true, message, '==', assert.ok);
}'
    at Function.AssertionError (assert.js:45:13)
    at repl:1:8
    at realRunInThisContextScript (vm.js:22:35)
    at sigintHandlersWrap (vm.js:98:12)
    at ContextifyScript.Script.runInThisContext (vm.js:24:12)
    at REPLServer.defaultEval (repl.js:346:29)
    at bound (domain.js:280:14)
    at REPLServer.runBound [as eval] (domain.js:293:12)
    at REPLServer.onLine (repl.js:545:10)
    at emitOne (events.js:101:20)
>
```

The `assert.AssertionError()` can only be used correctly with `new`,
so this converts it into a proper ES6 class that will give an
appropriate error message.

This also associates the appropriate internal/errors code with all
`assert.AssertionError` instances and updates the appropriate test
cases.

PR-URL: https://github.com/nodejs/node/pull/12651
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
This commit is contained in:
James M Snell 2017-04-25 13:18:25 -07:00
parent c1b3b95939
commit e48d58b8b2
12 changed files with 278 additions and 108 deletions

View File

@ -27,6 +27,13 @@ const { isSet, isMap } = process.binding('util');
const objectToString = require('internal/util').objectToString;
const Buffer = require('buffer').Buffer;
var errors;
function lazyErrors() {
if (!errors)
errors = require('internal/errors');
return errors;
}
// The assert module provides functions that throw
// AssertionError's when particular conditions are not met. The
// assert module must conform to the following interface.
@ -38,34 +45,33 @@ const assert = module.exports = ok;
// actual: actual,
// expected: expected });
assert.AssertionError = function AssertionError(options) {
this.name = 'AssertionError';
this.actual = options.actual;
this.expected = options.expected;
this.operator = options.operator;
if (options.message) {
this.message = options.message;
this.generatedMessage = false;
} else {
this.message = getMessage(this);
this.generatedMessage = true;
// TODO(jasnell): Consider moving AssertionError into internal/errors.js
class AssertionError extends Error {
constructor(options = {}) {
if (typeof options !== 'object' || options === null) {
// Lazy because the errors module itself uses assertions, leading to
// a circular dependency. This can be eliminated by moving this class
// into internal/errors.js
const errors = lazyErrors();
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'options', 'object');
}
const message = options.message ||
`${util.inspect(options.actual).slice(0, 128)} ` +
`${options.operator} ` +
util.inspect(options.expected).slice(0, 128);
super(message);
this.generatedMessage = !options.message;
this.name = 'AssertionError [ERR_ASSERTION]';
this.code = 'ERR_ASSERTION';
this.actual = options.actual;
this.expected = options.expected;
this.operator = options.operator;
var stackStartFunction = options.stackStartFunction || fail;
Error.captureStackTrace(this, stackStartFunction);
}
var stackStartFunction = options.stackStartFunction || fail;
Error.captureStackTrace(this, stackStartFunction);
};
// assert.AssertionError instanceof Error
util.inherits(assert.AssertionError, Error);
function truncate(s, n) {
return s.slice(0, n);
}
function getMessage(self) {
return truncate(util.inspect(self.actual), 128) + ' ' +
self.operator + ' ' +
truncate(util.inspect(self.expected), 128);
}
assert.AssertionError = AssertionError;
// At present only the three keys mentioned above are used and
// understood by the spec. Implementations or sub modules can pass
@ -83,7 +89,7 @@ function fail(actual, expected, message, operator, stackStartFunction) {
message = actual;
if (arguments.length === 2)
operator = '!=';
throw new assert.AssertionError({
throw new AssertionError({
message: message,
actual: actual,
expected: expected,

View File

@ -1,9 +1,9 @@
Exiting with code=1
assert.js:*
throw new assert.AssertionError({
throw new AssertionError({
^
AssertionError: 1 === 2
AssertionError [ERR_ASSERTION]: 1 === 2
at Object.<anonymous> (*test*message*error_exit.js:*:*)
at Module._compile (module.js:*:*)
at Object.Module._extensions..js (module.js:*:*)

View File

@ -1,5 +1,5 @@
'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');
const util = require('util');
@ -13,7 +13,10 @@ function re(literals, ...values) {
result += str.replace(/[\\^$.*+?()[\]{}|=!<>:-]/g, '\\$&');
result += literals[i + 1];
}
return new RegExp('^AssertionError: ' + result + '$');
return common.expectsError({
code: 'ERR_ASSERTION',
message: new RegExp(`^${result}$`)
});
}
// Turn off no-restricted-properties because we are testing deepEqual!

View File

@ -1,5 +1,5 @@
'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');
const util = require('util');
@ -13,7 +13,10 @@ function re(literals, ...values) {
result += str.replace(/[\\^$.*+?()[\]{}|=!<>:-]/g, '\\$&');
result += literals[i + 1];
}
return new RegExp(`^AssertionError: ${result}$`);
return common.expectsError({
code: 'ERR_ASSERTION',
message: new RegExp(`^${result}$`)
});
}
// The following deepEqual tests might seem very weird.
@ -112,8 +115,10 @@ for (const a of similar) {
assert.throws(
() => { assert.deepEqual(new Set([{a: 0}]), new Set([{a: 1}])); },
/^AssertionError: Set { { a: 0 } } deepEqual Set { { a: 1 } }$/
);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^Set { { a: 0 } } deepEqual Set { { a: 1 } }$/
}));
function assertDeepAndStrictEqual(a, b) {
assert.deepEqual(a, b);

View File

@ -1,33 +1,53 @@
'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');
// no args
assert.throws(
() => { assert.fail(); },
/^AssertionError: undefined undefined undefined$/
common.expectsError({
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'undefined undefined undefined'
})
);
// one arg = message
assert.throws(
() => { assert.fail('custom message'); },
/^AssertionError: custom message$/
common.expectsError({
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'custom message'
})
);
// two args only, operator defaults to '!='
assert.throws(
() => { assert.fail('first', 'second'); },
/^AssertionError: 'first' != 'second'$/
common.expectsError({
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: '\'first\' != \'second\''
})
);
// three args
assert.throws(
() => { assert.fail('ignored', 'ignored', 'another custom message'); },
/^AssertionError: another custom message$/
common.expectsError({
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: 'another custom message'
})
);
// no third arg (but a fourth arg)
assert.throws(
() => { assert.fail('first', 'second', undefined, 'operator'); },
/^AssertionError: 'first' operator 'second'$/
common.expectsError({
code: 'ERR_ASSERTION',
type: assert.AssertionError,
message: '\'first\' operator \'second\''
})
);

View File

@ -110,21 +110,44 @@ assert.doesNotThrow(makeBlock(a.deepEqual, /a/i, /a/i));
assert.doesNotThrow(makeBlock(a.deepEqual, /a/m, /a/m));
assert.doesNotThrow(makeBlock(a.deepEqual, /a/igm, /a/igm));
assert.throws(makeBlock(a.deepEqual, /ab/, /a/),
/^AssertionError: \/ab\/ deepEqual \/a\/$/);
common.expectsError({
code: 'ERR_ASSERTION',
type: a.AssertionError,
message: /^\/ab\/ deepEqual \/a\/$/
}));
assert.throws(makeBlock(a.deepEqual, /a/g, /a/),
/^AssertionError: \/a\/g deepEqual \/a\/$/);
common.expectsError({
code: 'ERR_ASSERTION',
type: a.AssertionError,
message: /^\/a\/g deepEqual \/a\/$/
}));
assert.throws(makeBlock(a.deepEqual, /a/i, /a/),
/^AssertionError: \/a\/i deepEqual \/a\/$/);
common.expectsError({
code: 'ERR_ASSERTION',
type: a.AssertionError,
message: /^\/a\/i deepEqual \/a\/$/
}));
assert.throws(makeBlock(a.deepEqual, /a/m, /a/),
/^AssertionError: \/a\/m deepEqual \/a\/$/);
common.expectsError({
code: 'ERR_ASSERTION',
type: a.AssertionError,
message: /^\/a\/m deepEqual \/a\/$/
}));
assert.throws(makeBlock(a.deepEqual, /a/igm, /a/im),
/^AssertionError: \/a\/gim deepEqual \/a\/im$/);
common.expectsError({
code: 'ERR_ASSERTION',
type: a.AssertionError,
message: /^\/a\/gim deepEqual \/a\/im$/
}));
{
const re1 = /a/g;
re1.lastIndex = 3;
assert.doesNotThrow(makeBlock(a.deepEqual, re1, /a/g),
/^AssertionError: \/a\/g deepEqual \/a\/g$/);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^\/a\/g deepEqual \/a\/g$/
}));
}
assert.doesNotThrow(makeBlock(a.deepEqual, 4, '4'), 'deepEqual(4, \'4\')');
@ -232,24 +255,39 @@ assert.doesNotThrow(makeBlock(a.deepStrictEqual, /a/m, /a/m));
assert.doesNotThrow(makeBlock(a.deepStrictEqual, /a/igm, /a/igm));
assert.throws(
makeBlock(a.deepStrictEqual, /ab/, /a/),
/^AssertionError: \/ab\/ deepStrictEqual \/a\/$/
);
common.expectsError({
code: 'ERR_ASSERTION',
type: a.AssertionError,
message: /^\/ab\/ deepStrictEqual \/a\/$/
}));
assert.throws(
makeBlock(a.deepStrictEqual, /a/g, /a/),
/^AssertionError: \/a\/g deepStrictEqual \/a\/$/
);
common.expectsError({
code: 'ERR_ASSERTION',
type: a.AssertionError,
message: /^\/a\/g deepStrictEqual \/a\/$/
}));
assert.throws(
makeBlock(a.deepStrictEqual, /a/i, /a/),
/^AssertionError: \/a\/i deepStrictEqual \/a\/$/
);
common.expectsError({
code: 'ERR_ASSERTION',
type: a.AssertionError,
message: /^\/a\/i deepStrictEqual \/a\/$/
}));
assert.throws(
makeBlock(a.deepStrictEqual, /a/m, /a/),
/^AssertionError: \/a\/m deepStrictEqual \/a\/$/
);
common.expectsError({
code: 'ERR_ASSERTION',
type: a.AssertionError,
message: /^\/a\/m deepStrictEqual \/a\/$/
}));
assert.throws(
makeBlock(a.deepStrictEqual, /a/igm, /a/im),
/^AssertionError: \/a\/gim deepStrictEqual \/a\/im$/
);
common.expectsError({
code: 'ERR_ASSERTION',
type: a.AssertionError,
message: /^\/a\/gim deepStrictEqual \/a\/im$/
}));
{
const re1 = /a/;
@ -275,11 +313,23 @@ assert.doesNotThrow(makeBlock(a.deepStrictEqual,
{a: 4, b: '2'},
{a: 4, b: '2'}));
assert.throws(makeBlock(a.deepStrictEqual, [4], ['4']),
/^AssertionError: \[ 4 ] deepStrictEqual \[ '4' ]$/);
common.expectsError({
code: 'ERR_ASSERTION',
type: a.AssertionError,
message: /^\[ 4 ] deepStrictEqual \[ '4' ]$/
}));
assert.throws(makeBlock(a.deepStrictEqual, {a: 4}, {a: 4, b: true}),
/^AssertionError: { a: 4 } deepStrictEqual { a: 4, b: true }$/);
common.expectsError({
code: 'ERR_ASSERTION',
type: a.AssertionError,
message: /^{ a: 4 } deepStrictEqual { a: 4, b: true }$/
}));
assert.throws(makeBlock(a.deepStrictEqual, ['a'], {0: 'a'}),
/^AssertionError: \[ 'a' ] deepStrictEqual { '0': 'a' }$/);
common.expectsError({
code: 'ERR_ASSERTION',
type: a.AssertionError,
message: /^\[ 'a' ] deepStrictEqual { '0': 'a' }$/
}));
//(although not necessarily the same order),
assert.doesNotThrow(makeBlock(a.deepStrictEqual,
{a: 4, b: '1'},
@ -349,7 +399,7 @@ assert.throws(makeBlock(a.deepStrictEqual, new Boolean(true), {}),
// Testing the throwing
function thrower(errorConstructor) {
throw new errorConstructor('test');
throw new errorConstructor({});
}
// the basic calls work
@ -430,11 +480,11 @@ assert.throws(() => {
}
// use a RegExp to validate error message
a.throws(makeBlock(thrower, TypeError), /test/);
a.throws(makeBlock(thrower, TypeError), /\[object Object\]/);
// use a fn to validate error object
a.throws(makeBlock(thrower, TypeError), function(err) {
if ((err instanceof TypeError) && /test/.test(err)) {
if ((err instanceof TypeError) && /\[object Object\]/.test(err)) {
return true;
}
});
@ -511,23 +561,31 @@ a.throws(makeBlock(a.deepEqual, args, []));
{
assert.throws(
() => { a.throws(common.noop); },
/^AssertionError: Missing expected exception\.$/
);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^Missing expected exception\.$/
}));
assert.throws(
() => { a.throws(common.noop, TypeError); },
/^AssertionError: Missing expected exception \(TypeError\)\.$/
);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^Missing expected exception \(TypeError\)\.$/
}));
assert.throws(
() => { a.throws(common.noop, 'fhqwhgads'); },
/^AssertionError: Missing expected exception: fhqwhgads$/
);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^Missing expected exception: fhqwhgads$/
}));
assert.throws(
() => { a.throws(common.noop, TypeError, 'fhqwhgads'); },
/^AssertionError: Missing expected exception \(TypeError\): fhqwhgads$/
);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^Missing expected exception \(TypeError\): fhqwhgads$/
}));
}
const circular = {y: 1};
@ -537,8 +595,8 @@ function testAssertionMessage(actual, expected) {
try {
assert.strictEqual(actual, '');
} catch (e) {
assert.strictEqual(e.toString(),
['AssertionError:', expected, '===', '\'\''].join(' '));
assert.strictEqual(e.message,
[expected, '===', '\'\''].join(' '));
assert.ok(e.generatedMessage, 'Message not marked as generated');
}
}
@ -585,14 +643,14 @@ testAssertionMessage({a: NaN, b: Infinity, c: -Infinity},
try {
assert.strictEqual(1, 2);
} catch (e) {
assert.strictEqual(e.toString().split('\n')[0], 'AssertionError: 1 === 2');
assert.strictEqual(e.message.split('\n')[0], '1 === 2');
assert.ok(e.generatedMessage, 'Message not marked as generated');
}
try {
assert.strictEqual(1, 2, 'oh no');
} catch (e) {
assert.strictEqual(e.toString().split('\n')[0], 'AssertionError: oh no');
assert.strictEqual(e.message.split('\n')[0], 'oh no');
assert.strictEqual(e.generatedMessage, false,
'Message incorrectly marked as generated');
}
@ -639,6 +697,16 @@ assert.throws(() => { throw new Error(); }, (err) => err instanceof Error);
// Long values should be truncated for display.
assert.throws(() => {
assert.strictEqual('A'.repeat(1000), '');
}, new RegExp(`^AssertionError: '${'A'.repeat(127)} === ''$`));
}, common.expectsError({
code: 'ERR_ASSERTION',
message: new RegExp(`^'${'A'.repeat(127)} === ''$`)}));
console.log('All OK');
[1, true, false, '', null, Infinity, Symbol('test')].forEach((input) => {
assert.throws(
() => new assert.AssertionError(input),
common.expectsError({
code: 'ERR_INVALID_ARG_TYPE',
type: TypeError,
message: /^The "options" argument must be of type object$/
}));
});

View File

@ -43,5 +43,7 @@ assert.throws(function() {
// assert.fail() tests
assert.throws(
() => { assert.fail('fhqwhgads'); },
/^AssertionError: fhqwhgads$/
);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^fhqwhgads$/
}));

View File

@ -151,7 +151,10 @@ assert.strictEqual(errStrings.length, 0);
assert.throws(() => {
console.assert(false, 'should throw');
}, /^AssertionError: should throw$/);
}, common.expectsError({
code: 'ERR_ASSERTION',
message: /^should throw$/
}));
assert.doesNotThrow(() => {
console.assert(true, 'this should not throw');

View File

@ -8,7 +8,10 @@ const _createSocketHandle = dgram._createSocketHandle;
// Throws if an "existing fd" is passed in.
assert.throws(() => {
_createSocketHandle(common.localhostIPv4, 0, 'udp4', 42);
}, /^AssertionError: false == true$/);
}, common.expectsError({
code: 'ERR_ASSERTION',
message: /^false == true$/
}));
{
// Create a handle that is not bound.

View File

@ -41,47 +41,89 @@ assert.strictEqual(err5.code, 'TEST_ERROR_1');
assert.throws(
() => new errors.Error('TEST_FOO_KEY'),
/^AssertionError: An invalid error message key was used: TEST_FOO_KEY\.$/);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^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\.$/);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^An invalid error message key was used: TEST_FOO_KEY\.$/
}));
assert.throws(
() => new errors.Error(1),
/^AssertionError: 'number' === 'string'$/);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^'number' === 'string'$/
}));
assert.throws(
() => new errors.Error({}),
/^AssertionError: 'object' === 'string'$/);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^'object' === 'string'$/
}));
assert.throws(
() => new errors.Error([]),
/^AssertionError: 'object' === 'string'$/);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^'object' === 'string'$/
}));
assert.throws(
() => new errors.Error(true),
/^AssertionError: 'boolean' === 'string'$/);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^'boolean' === 'string'$/
}));
assert.throws(
() => new errors.TypeError(1),
/^AssertionError: 'number' === 'string'$/);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^'number' === 'string'$/
}));
assert.throws(
() => new errors.TypeError({}),
/^AssertionError: 'object' === 'string'$/);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^'object' === 'string'$/
}));
assert.throws(
() => new errors.TypeError([]),
/^AssertionError: 'object' === 'string'$/);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^'object' === 'string'$/
}));
assert.throws(
() => new errors.TypeError(true),
/^AssertionError: 'boolean' === 'string'$/);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^'boolean' === 'string'$/
}));
assert.throws(
() => new errors.RangeError(1),
/^AssertionError: 'number' === 'string'$/);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^'number' === 'string'$/
}));
assert.throws(
() => new errors.RangeError({}),
/^AssertionError: 'object' === 'string'$/);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^'object' === 'string'$/
}));
assert.throws(
() => new errors.RangeError([]),
/^AssertionError: 'object' === 'string'$/);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^'object' === 'string'$/
}));
assert.throws(
() => new errors.RangeError(true),
/^AssertionError: 'boolean' === 'string'$/);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^'boolean' === 'string'$/
}));
// Tests for common.expectsError
@ -115,7 +157,10 @@ assert.throws(() => {
assert.throws(() => {
throw new errors.TypeError('TEST_ERROR_1', 'a');
}, common.expectsError({ code: 'TEST_ERROR_1', type: RangeError }));
}, /^AssertionError: .+ is not the expected type \S/);
}, common.expectsError({
code: 'ERR_ASSERTION',
message: /^.+ is not the expected type \S/
}));
assert.throws(() => {
assert.throws(() => {
@ -123,7 +168,10 @@ assert.throws(() => {
}, common.expectsError({ code: 'TEST_ERROR_1',
type: TypeError,
message: /^Error for testing 2/ }));
}, /AssertionError: .+ does not match \S/);
}, common.expectsError({
code: 'ERR_ASSERTION',
message: /.+ does not match \S/
}));
// // Test ERR_INVALID_ARG_TYPE
assert.strictEqual(errors.message('ERR_INVALID_ARG_TYPE', ['a', 'b']),
@ -156,8 +204,10 @@ assert.strictEqual(errors.message('ERR_INVALID_URL_SCHEME', [['a', 'b', 'c']]),
'The URL must be one of scheme a, b, or c');
assert.throws(
() => errors.message('ERR_INVALID_URL_SCHEME', [[]]),
/^AssertionError: At least one expected value needs to be specified$/
);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^At least one expected value needs to be specified$/
}));
// Test ERR_MISSING_ARGS
assert.strictEqual(errors.message('ERR_MISSING_ARGS', ['name']),
@ -168,5 +218,7 @@ assert.strictEqual(errors.message('ERR_MISSING_ARGS', ['a', 'b', 'c']),
'The "a", "b", and "c" arguments must be specified');
assert.throws(
() => errors.message('ERR_MISSING_ARGS'),
/^AssertionError: At least one arg needs to be specified$/
);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^At least one arg needs to be specified$/
}));

View File

@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.
'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');
const error_desc = {
@ -44,10 +44,14 @@ assert.throws(
assert.throws(
require,
/^AssertionError: missing path$/
);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^missing path$/
}));
assert.throws(
() => { require({}); },
/^AssertionError: path must be a string$/
);
common.expectsError({
code: 'ERR_ASSERTION',
message: /^path must be a string$/
}));

View File

@ -1,5 +1,5 @@
'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');
const { Readable, Writable, Duplex, Transform } = require('stream');
@ -42,4 +42,8 @@ Object.setPrototypeOf(CustomWritable.prototype, Writable.prototype);
new CustomWritable();
assert.throws(CustomWritable, /AssertionError: inherits from Writable/);
assert.throws(CustomWritable,
common.expectsError({
code: 'ERR_ASSERTION',
message: /^inherits from Writable$/
}));