Skip to content

Commit ef8f147

Browse files
committed
assert: improve regular expression validation
This makes sure `assert.throws()` and `assert.rejects()` result in an easy to understand error message instead of rethrowing the actual error. This should significantly improve the debugging experience in case people use an regular expression to validate their errors. This also adds support for primitive errors that would have caused runtime errors using the mentioned functions. The input is now stringified before it's passed to the RegExp to circumvent that. As drive-by change this also adds some further comments and renames a variable for clarity. PR-URL: #27781 Reviewed-By: Rich Trott <[email protected]>
1 parent 8157a50 commit ef8f147

File tree

2 files changed

+75
-18
lines changed

2 files changed

+75
-18
lines changed

‎lib/assert.js‎

Lines changed: 54 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -549,30 +549,38 @@ function compareExceptionKey(actual, expected, key, message, keys, fn){
549549
}
550550
}
551551

552-
functionexpectedException(actual,expected,msg,fn){
552+
functionexpectedException(actual,expected,message,fn){
553553
if(typeofexpected!=='function'){
554-
if(isRegExp(expected))
555-
returnexpected.test(actual);
556-
// assert.doesNotThrow does not accept objects.
557-
if(arguments.length===2){
558-
thrownewERR_INVALID_ARG_TYPE(
559-
'expected',['Function','RegExp'],expected
560-
);
554+
// Handle regular expressions.
555+
if(isRegExp(expected)){
556+
conststr=String(actual);
557+
if(expected.test(str))
558+
return;
559+
560+
thrownewAssertionError({
561+
actual,
562+
expected,
563+
message: message||'The input did not match the regular expression '+
564+
`${inspect(expected)}. Input:\n\n${inspect(str)}\n`,
565+
operator: fn.name,
566+
stackStartFn: fn
567+
});
561568
}
562569

563570
// Handle primitives properly.
564571
if(typeofactual!=='object'||actual===null){
565572
consterr=newAssertionError({
566573
actual,
567574
expected,
568-
message: msg,
575+
message,
569576
operator: 'deepStrictEqual',
570577
stackStartFn: fn
571578
});
572579
err.operator=fn.name;
573580
throwerr;
574581
}
575582

583+
// Handle validation objects.
576584
constkeys=Object.keys(expected);
577585
// Special handle errors to make sure the name and the message are compared
578586
// as well.
@@ -589,18 +597,25 @@ function expectedException(actual, expected, msg, fn){
589597
expected[key].test(actual[key])){
590598
continue;
591599
}
592-
compareExceptionKey(actual,expected,key,msg,keys,fn);
600+
compareExceptionKey(actual,expected,key,message,keys,fn);
593601
}
594-
returntrue;
602+
return;
595603
}
604+
596605
// Guard instanceof against arrow functions as they don't have a prototype.
606+
// Check for matching Error classes.
597607
if(expected.prototype!==undefined&&actualinstanceofexpected){
598-
returntrue;
608+
return;
599609
}
600610
if(Error.isPrototypeOf(expected)){
601-
returnfalse;
611+
throwactual;
612+
}
613+
614+
// Check validation functions return value.
615+
constres=expected.call({},actual);
616+
if(res!==true){
617+
throwactual;
602618
}
603-
returnexpected.call({},actual)===true;
604619
}
605620

606621
functiongetActual(fn){
@@ -695,9 +710,31 @@ function expectsError(stackStartFn, actual, error, message){
695710
stackStartFn
696711
});
697712
}
698-
if(error&&!expectedException(actual,error,message,stackStartFn)){
699-
throwactual;
713+
714+
if(!error)
715+
return;
716+
717+
expectedException(actual,error,message,stackStartFn);
718+
}
719+
720+
functionhasMatchingError(actual,expected){
721+
if(typeofexpected!=='function'){
722+
if(isRegExp(expected)){
723+
conststr=String(actual);
724+
returnexpected.test(str);
725+
}
726+
thrownewERR_INVALID_ARG_TYPE(
727+
'expected',['Function','RegExp'],expected
728+
);
700729
}
730+
// Guard instanceof against arrow functions as they don't have a prototype.
731+
if(expected.prototype!==undefined&&actualinstanceofexpected){
732+
returntrue;
733+
}
734+
if(Error.isPrototypeOf(expected)){
735+
returnfalse;
736+
}
737+
returnexpected.call({},actual)===true;
701738
}
702739

703740
functionexpectsNoError(stackStartFn,actual,error,message){
@@ -709,7 +746,7 @@ function expectsNoError(stackStartFn, actual, error, message){
709746
error=undefined;
710747
}
711748

712-
if(!error||expectedException(actual,error)){
749+
if(!error||hasMatchingError(actual,error)){
713750
constdetails=message ? `: ${message}` : '.';
714751
constfnType=stackStartFn.name==='doesNotReject' ?
715752
'rejection' : 'exception';

‎test/parallel/test-assert.js‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,27 @@ assert.throws(
182182
}
183183

184184
// Use a RegExp to validate the error message.
185-
a.throws(()=>thrower(TypeError),/\[objectObject\]/);
185+
{
186+
a.throws(()=>thrower(TypeError),/\[objectObject\]/);
187+
188+
constsymbol=Symbol('foo');
189+
a.throws(()=>{
190+
throwsymbol;
191+
},/foo/);
192+
193+
a.throws(()=>{
194+
a.throws(()=>{
195+
throwsymbol;
196+
},/abc/);
197+
},{
198+
message: 'The input did not match the regular expression /abc/. '+
199+
"Input:\n\n'Symbol(foo)'\n",
200+
code: 'ERR_ASSERTION',
201+
operator: 'throws',
202+
actual: symbol,
203+
expected: /abc/
204+
});
205+
}
186206

187207
// Use a fn to validate the error object.
188208
a.throws(()=>thrower(TypeError),(err)=>{

0 commit comments

Comments
(0)