Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 147aeed

Browse filesBrowse files
BridgeARMylesBorins
BridgeAR
authored and
MylesBorins
committed
assert: improve assert.throws
Throw a TypeError in case a error message is provided in the second argument and a third argument is present as well. This is clearly a mistake and should not be done. Backport-PR-URL: #23223 PR-URL: #17585 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
1 parent c9d84b6 commit 147aeed
Copy full SHA for 147aeed

File tree

Expand file treeCollapse file tree

3 files changed

+93
-52
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

3 files changed

+93
-52
lines changed
Open diff view settings
Collapse file

‎doc/api/assert.md‎

Copy file name to clipboardExpand all lines: doc/api/assert.md
+31-6Lines changed: 31 additions & 6 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -691,17 +691,42 @@ assert.throws(
691691

692692
Note that `error` can not be a string. If a string is provided as the second
693693
argument, then `error` is assumed to be omitted and the string will be used for
694-
`message` instead. This can lead to easy-to-miss mistakes:
694+
`message` instead. This can lead to easy-to-miss mistakes. Please read the
695+
example below carefully if using a string as the second argument gets
696+
considered:
695697

696698
<!-- eslint-disable no-restricted-syntax -->
697699
```js
698-
// THIS IS A MISTAKE! DO NOT DO THIS!
699-
assert.throws(myFunction, 'missing foo', 'did not throw with expected message');
700-
701-
// Do this instead.
702-
assert.throws(myFunction, /missing foo/, 'did not throw with expected message');
700+
function throwingFirst() {
701+
throw new Error('First');
702+
}
703+
function throwingSecond() {
704+
throw new Error('Second');
705+
}
706+
function notThrowing() {}
707+
708+
// The second argument is a string and the input function threw an Error.
709+
// In that case both cases do not throw as neither is going to try to
710+
// match for the error message thrown by the input function!
711+
assert.throws(throwingFirst, 'Second');
712+
assert.throws(throwingSecond, 'Second');
713+
714+
// The string is only used (as message) in case the function does not throw:
715+
assert.throws(notThrowing, 'Second');
716+
// AssertionError [ERR_ASSERTION]: Missing expected exception: Second
717+
718+
// If it was intended to match for the error message do this instead:
719+
assert.throws(throwingSecond, /Second$/);
720+
// Does not throw because the error messages match.
721+
assert.throws(throwingFirst, /Second$/);
722+
// Throws a error:
723+
// Error: First
724+
// at throwingFirst (repl:2:9)
703725
```
704726

727+
Due to the confusing notation, it is recommended not to use a string as the
728+
second argument. This might lead to difficult-to-spot errors.
729+
705730
## Caveats
706731

707732
For the following cases, consider using ES2015 [`Object.is()`][],
Collapse file

‎lib/assert.js‎

Copy file name to clipboardExpand all lines: lib/assert.js
+50-45Lines changed: 50 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -675,68 +675,73 @@ function expectedException(actual, expected) {
675675
return expected.call({}, actual) === true;
676676
}
677677

678-
function tryBlock(block) {
678+
function getActual(block) {
679+
if (typeof block !== 'function') {
680+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'block', 'Function',
681+
block);
682+
}
679683
try {
680684
block();
681685
} catch (e) {
682686
return e;
683687
}
684688
}
685689

686-
function innerThrows(shouldThrow, block, expected, message) {
687-
var details = '';
690+
// Expected to throw an error.
691+
assert.throws = function throws(block, error, message) {
692+
const actual = getActual(block);
688693

689-
if (typeof block !== 'function') {
690-
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'block', 'function',
691-
block);
692-
}
694+
if (typeof error === 'string') {
695+
if (arguments.length === 3)
696+
throw new errors.TypeError('ERR_INVALID_ARG_TYPE',
697+
'error',
698+
['Function', 'RegExp'],
699+
error);
693700

694-
if (typeof expected === 'string') {
695-
message = expected;
696-
expected = null;
701+
message = error;
702+
error = null;
697703
}
698704

699-
const actual = tryBlock(block);
700-
701-
if (shouldThrow === true) {
702-
if (actual === undefined) {
703-
if (expected && expected.name) {
704-
details += ` (${expected.name})`;
705-
}
706-
details += message ? `: ${message}` : '.';
707-
innerFail({
708-
actual,
709-
expected,
710-
operator: 'throws',
711-
message: `Missing expected exception${details}`,
712-
stackStartFn: assert.throws
713-
});
714-
}
715-
if (expected && expectedException(actual, expected) === false) {
716-
throw actual;
717-
}
718-
} else if (actual !== undefined) {
719-
if (!expected || expectedException(actual, expected)) {
720-
details = message ? `: ${message}` : '.';
721-
innerFail({
722-
actual,
723-
expected,
724-
operator: 'doesNotThrow',
725-
message: `Got unwanted exception${details}`,
726-
stackStartFn: assert.doesNotThrow
727-
});
705+
if (actual === undefined) {
706+
let details = '';
707+
if (error && error.name) {
708+
details += ` (${error.name})`;
728709
}
710+
details += message ? `: ${message}` : '.';
711+
innerFail({
712+
actual,
713+
expected: error,
714+
operator: 'throws',
715+
message: `Missing expected exception${details}`,
716+
stackStartFn: throws
717+
});
718+
}
719+
if (error && expectedException(actual, error) === false) {
729720
throw actual;
730721
}
731-
}
732-
733-
// Expected to throw an error.
734-
assert.throws = function throws(block, error, message) {
735-
innerThrows(true, block, error, message);
736722
};
737723

738724
assert.doesNotThrow = function doesNotThrow(block, error, message) {
739-
innerThrows(false, block, error, message);
725+
const actual = getActual(block);
726+
if (actual === undefined)
727+
return;
728+
729+
if (typeof error === 'string') {
730+
message = error;
731+
error = null;
732+
}
733+
734+
if (!error || expectedException(actual, error)) {
735+
const details = message ? `: ${message}` : '.';
736+
innerFail({
737+
actual,
738+
expected: error,
739+
operator: 'doesNotThrow',
740+
message: `Got unwanted exception${details}\n${actual.message}`,
741+
stackStartFn: doesNotThrow
742+
});
743+
}
744+
throw actual;
740745
};
741746

742747
assert.ifError = function ifError(err) { if (err) throw err; };
Collapse file

‎test/parallel/test-assert.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-assert.js
+12-1Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -640,7 +640,7 @@ try {
640640
common.expectsError({
641641
code: 'ERR_INVALID_ARG_TYPE',
642642
type: TypeError,
643-
message: 'The "block" argument must be of type function. Received ' +
643+
message: 'The "block" argument must be of type Function. Received ' +
644644
`type ${typeName(block)}`
645645
})(e);
646646
}
@@ -731,3 +731,14 @@ common.expectsError(
731731
message: 'null == true'
732732
}
733733
);
734+
735+
common.expectsError(
736+
// eslint-disable-next-line no-restricted-syntax
737+
() => assert.throws(() => {}, 'Error message', 'message'),
738+
{
739+
code: 'ERR_INVALID_ARG_TYPE',
740+
type: TypeError,
741+
message: 'The "error" argument must be one of type Function or RegExp. ' +
742+
'Received type string'
743+
}
744+
);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.