Skip to content

Commit e48d58b

Browse files
committed
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: #12651 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
1 parent c1b3b95 commit e48d58b

12 files changed

+279
-109
lines changed

lib/assert.js

+33-27
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@ const { isSet, isMap } = process.binding('util');
2727
const objectToString = require('internal/util').objectToString;
2828
const Buffer = require('buffer').Buffer;
2929

30+
var errors;
31+
function lazyErrors() {
32+
if (!errors)
33+
errors = require('internal/errors');
34+
return errors;
35+
}
36+
3037
// The assert module provides functions that throw
3138
// AssertionError's when particular conditions are not met. The
3239
// assert module must conform to the following interface.
@@ -38,34 +45,33 @@ const assert = module.exports = ok;
3845
// actual: actual,
3946
// expected: expected });
4047

41-
assert.AssertionError = function AssertionError(options) {
42-
this.name = 'AssertionError';
43-
this.actual = options.actual;
44-
this.expected = options.expected;
45-
this.operator = options.operator;
46-
if (options.message) {
47-
this.message = options.message;
48-
this.generatedMessage = false;
49-
} else {
50-
this.message = getMessage(this);
51-
this.generatedMessage = true;
52-
}
53-
var stackStartFunction = options.stackStartFunction || fail;
54-
Error.captureStackTrace(this, stackStartFunction);
55-
};
56-
57-
// assert.AssertionError instanceof Error
58-
util.inherits(assert.AssertionError, Error);
59-
60-
function truncate(s, n) {
61-
return s.slice(0, n);
48+
// TODO(jasnell): Consider moving AssertionError into internal/errors.js
49+
class AssertionError extends Error {
50+
constructor(options = {}) {
51+
if (typeof options !== 'object' || options === null) {
52+
// Lazy because the errors module itself uses assertions, leading to
53+
// a circular dependency. This can be eliminated by moving this class
54+
// into internal/errors.js
55+
const errors = lazyErrors();
56+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'options', 'object');
57+
}
58+
const message = options.message ||
59+
`${util.inspect(options.actual).slice(0, 128)} ` +
60+
`${options.operator} ` +
61+
util.inspect(options.expected).slice(0, 128);
62+
super(message);
63+
this.generatedMessage = !options.message;
64+
this.name = 'AssertionError [ERR_ASSERTION]';
65+
this.code = 'ERR_ASSERTION';
66+
this.actual = options.actual;
67+
this.expected = options.expected;
68+
this.operator = options.operator;
69+
var stackStartFunction = options.stackStartFunction || fail;
70+
Error.captureStackTrace(this, stackStartFunction);
71+
}
6272
}
6373

64-
function getMessage(self) {
65-
return truncate(util.inspect(self.actual), 128) + ' ' +
66-
self.operator + ' ' +
67-
truncate(util.inspect(self.expected), 128);
68-
}
74+
assert.AssertionError = AssertionError;
6975

7076
// At present only the three keys mentioned above are used and
7177
// understood by the spec. Implementations or sub modules can pass
@@ -83,7 +89,7 @@ function fail(actual, expected, message, operator, stackStartFunction) {
8389
message = actual;
8490
if (arguments.length === 2)
8591
operator = '!=';
86-
throw new assert.AssertionError({
92+
throw new AssertionError({
8793
message: message,
8894
actual: actual,
8995
expected: expected,

test/message/error_exit.out

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
Exiting with code=1
2-
32
assert.js:*
4-
throw new assert.AssertionError({
3+
throw new AssertionError({
54
^
6-
AssertionError: 1 === 2
5+
6+
AssertionError [ERR_ASSERTION]: 1 === 2
77
at Object.<anonymous> (*test*message*error_exit.js:*:*)
88
at Module._compile (module.js:*:*)
99
at Object.Module._extensions..js (module.js:*:*)

test/parallel/test-assert-checktag.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
'use strict';
2-
require('../common');
2+
const common = require('../common');
33
const assert = require('assert');
44
const util = require('util');
55

@@ -13,7 +13,10 @@ function re(literals, ...values) {
1313
result += str.replace(/[\\^$.*+?()[\]{}|=!<>:-]/g, '\\$&');
1414
result += literals[i + 1];
1515
}
16-
return new RegExp('^AssertionError: ' + result + '$');
16+
return common.expectsError({
17+
code: 'ERR_ASSERTION',
18+
message: new RegExp(`^${result}$`)
19+
});
1720
}
1821

1922
// Turn off no-restricted-properties because we are testing deepEqual!

test/parallel/test-assert-deep.js

+9-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
'use strict';
2-
require('../common');
2+
const common = require('../common');
33
const assert = require('assert');
44
const util = require('util');
55

@@ -13,7 +13,10 @@ function re(literals, ...values) {
1313
result += str.replace(/[\\^$.*+?()[\]{}|=!<>:-]/g, '\\$&');
1414
result += literals[i + 1];
1515
}
16-
return new RegExp(`^AssertionError: ${result}$`);
16+
return common.expectsError({
17+
code: 'ERR_ASSERTION',
18+
message: new RegExp(`^${result}$`)
19+
});
1720
}
1821

1922
// The following deepEqual tests might seem very weird.
@@ -112,8 +115,10 @@ for (const a of similar) {
112115

113116
assert.throws(
114117
() => { assert.deepEqual(new Set([{a: 0}]), new Set([{a: 1}])); },
115-
/^AssertionError: Set { { a: 0 } } deepEqual Set { { a: 1 } }$/
116-
);
118+
common.expectsError({
119+
code: 'ERR_ASSERTION',
120+
message: /^Set { { a: 0 } } deepEqual Set { { a: 1 } }$/
121+
}));
117122

118123
function assertDeepAndStrictEqual(a, b) {
119124
assert.deepEqual(a, b);

test/parallel/test-assert-fail.js

+26-6
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,53 @@
11
'use strict';
2-
require('../common');
2+
const common = require('../common');
33
const assert = require('assert');
44

55
// no args
66
assert.throws(
77
() => { assert.fail(); },
8-
/^AssertionError: undefined undefined undefined$/
8+
common.expectsError({
9+
code: 'ERR_ASSERTION',
10+
type: assert.AssertionError,
11+
message: 'undefined undefined undefined'
12+
})
913
);
1014

1115
// one arg = message
1216
assert.throws(
1317
() => { assert.fail('custom message'); },
14-
/^AssertionError: custom message$/
18+
common.expectsError({
19+
code: 'ERR_ASSERTION',
20+
type: assert.AssertionError,
21+
message: 'custom message'
22+
})
1523
);
1624

1725
// two args only, operator defaults to '!='
1826
assert.throws(
1927
() => { assert.fail('first', 'second'); },
20-
/^AssertionError: 'first' != 'second'$/
28+
common.expectsError({
29+
code: 'ERR_ASSERTION',
30+
type: assert.AssertionError,
31+
message: '\'first\' != \'second\''
32+
})
2133
);
2234

2335
// three args
2436
assert.throws(
2537
() => { assert.fail('ignored', 'ignored', 'another custom message'); },
26-
/^AssertionError: another custom message$/
38+
common.expectsError({
39+
code: 'ERR_ASSERTION',
40+
type: assert.AssertionError,
41+
message: 'another custom message'
42+
})
2743
);
2844

2945
// no third arg (but a fourth arg)
3046
assert.throws(
3147
() => { assert.fail('first', 'second', undefined, 'operator'); },
32-
/^AssertionError: 'first' operator 'second'$/
48+
common.expectsError({
49+
code: 'ERR_ASSERTION',
50+
type: assert.AssertionError,
51+
message: '\'first\' operator \'second\''
52+
})
3353
);

0 commit comments

Comments
 (0)