Skip to content

Commit bb857d9

Browse files
BridgeARtargos
authored andcommitted
assert: make sure throws is able to handle primitives
This fixes some possible issues with `assert.throws` and `assert.rejects` in combination with an validation object. It will now properly handle primitive values being thrown as error. It also makes sure the `generatedMessage` property is properly set if `assert.throws` or `assert.rejects` is used in combination with an validation object and improves the error performance in such cases by only creating the error once. In addition it will fix detecting regular expressions from a different context such as n-api that are passed through as validator for `assert.throws` or `assert.rejects`. Until now those were not tested. PR-URL: #20482 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>
1 parent e93726a commit bb857d9

3 files changed

Lines changed: 72 additions & 7 deletions

File tree

‎lib/assert.js‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -382,16 +382,16 @@ function compareExceptionKey(actual, expected, key, message, keys) {
382382
consta=newComparison(actual,keys);
383383
constb=newComparison(expected,keys,actual);
384384

385-
consttmpLimit=Error.stackTraceLimit;
386-
Error.stackTraceLimit=0;
387385
consterr=newAssertionError({
388386
actual: a,
389387
expected: b,
390388
operator: 'deepStrictEqual',
391389
stackStartFn: assert.throws
392390
});
393-
Error.stackTraceLimit=tmpLimit;
394-
message=err.message;
391+
err.actual=actual;
392+
err.expected=expected;
393+
err.operator='throws';
394+
throwerr;
395395
}
396396
innerFail({
397397
actual,
@@ -405,14 +405,34 @@ function compareExceptionKey(actual, expected, key, message, keys) {
405405

406406
functionexpectedException(actual,expected,msg){
407407
if(typeofexpected!=='function'){
408-
if(expectedinstanceofRegExp)
408+
if(isRegExp(expected))
409409
returnexpected.test(actual);
410410
// assert.doesNotThrow does not accept objects.
411411
if(arguments.length===2){
412412
thrownewERR_INVALID_ARG_TYPE(
413413
'expected',['Function','RegExp'],expected
414414
);
415415
}
416+
417+
// TODO: Disallow primitives as error argument.
418+
// This is here to prevent a breaking change.
419+
if(typeofexpected!=='object'){
420+
returntrue;
421+
}
422+
423+
// Handle primitives properly.
424+
if(typeofactual!=='object'||actual===null){
425+
consterr=newAssertionError({
426+
actual,
427+
expected,
428+
message: msg,
429+
operator: 'deepStrictEqual',
430+
stackStartFn: assert.throws
431+
});
432+
err.operator='throws';
433+
throwerr;
434+
}
435+
416436
constkeys=Object.keys(expected);
417437
// Special handle errors to make sure the name and the message are compared
418438
// as well.

‎test/message/assert_throws_stack.out‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
assert.js:*
2-
throw new AssertionError(obj);
3-
^
2+
throw err;
3+
^
44

55
AssertionError [ERR_ASSERTION]: Input A expected to strictly deep-equal input B:
66
+ expected - actual

‎test/parallel/test-assert.js‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -740,7 +740,9 @@ common.expectsError(
740740
constframes=err.stack.split('\n');
741741
const[,filename,line,column]=frames[1].match(/\((.+):(\d+):(\d+)\)/);
742742
// Reset the cache to check again
743+
constsize=errorCache.size;
743744
errorCache.delete(`${filename}${line-1}${column-1}`);
745+
assert.strictEqual(errorCache.size,size-1);
744746
constdata=`${'\n'.repeat(line-1)}${' '.repeat(column-1)}`+
745747
'ok(failed(badly));';
746748
try{
@@ -849,6 +851,7 @@ common.expectsError(
849851
{
850852
name: 'AssertionError [ERR_ASSERTION]',
851853
code: 'ERR_ASSERTION',
854+
generatedMessage: true,
852855
message: `${start}\n${actExp}\n\n`+
853856
" Comparison {\n name: 'Error',\n- message: 'foo'"+
854857
"\n+ message: ''\n }"
@@ -940,3 +943,45 @@ assert.throws(
940943
' }'
941944
}
942945
);
946+
947+
{
948+
letactual=null;
949+
constexpected={message: 'foo'};
950+
assert.throws(
951+
()=>assert.throws(
952+
()=>{throwactual;},
953+
expected
954+
),
955+
{
956+
operator: 'throws',
957+
actual,
958+
expected,
959+
generatedMessage: true,
960+
message: `${start}\n${actExp}\n\n`+
961+
'- null\n'+
962+
'+ {\n'+
963+
"+ message: 'foo'\n"+
964+
'+ }'
965+
}
966+
);
967+
968+
actual='foobar';
969+
constmessage='message';
970+
assert.throws(
971+
()=>assert.throws(
972+
()=>{throwactual;},
973+
{message: 'foobar'},
974+
message
975+
),
976+
{
977+
actual,
978+
message,
979+
operator: 'throws',
980+
generatedMessage: false
981+
}
982+
);
983+
}
984+
985+
// TODO: This case is only there to make sure there is no breaking change.
986+
// eslint-disable-next-line no-restricted-syntax, no-throw-literal
987+
assert.throws(()=>{throw4;},4);

0 commit comments

Comments
 (0)