Skip to content

Commit

Permalink
assert: fix AssertionError, assign error code
Browse files Browse the repository at this point in the history
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: #12651
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
  • Loading branch information
jasnell committed May 4, 2017
1 parent c1b3b95 commit e48d58b
Show file tree
Hide file tree
Showing 12 changed files with 279 additions and 109 deletions.
60 changes: 33 additions & 27 deletions lib/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;
}
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);
// 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);
}
}

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
Expand All @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions test/message/error_exit.out
Original file line number Diff line number Diff line change
@@ -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:*:*)
Expand Down
7 changes: 5 additions & 2 deletions test/parallel/test-assert-checktag.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');
const util = require('util');

Expand All @@ -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!
Expand Down
13 changes: 9 additions & 4 deletions test/parallel/test-assert-deep.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
require('../common');
const common = require('../common');
const assert = require('assert');
const util = require('util');

Expand All @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
32 changes: 26 additions & 6 deletions test/parallel/test-assert-fail.js
Original file line number Diff line number Diff line change
@@ -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\''
})
);
Loading

0 comments on commit e48d58b

Please sign in to comment.