Skip to content

assert: fix exception message for assert(0) on try catch block #46760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nodejs-github-bot merged 4 commits into from
Feb 26, 2023
+78 −19
Merged

assert: fix exception message for assert(0) on try catch block #46760

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
76fbda3
assert: fix exception message for assert(0) on try catch block
Feb 21, 2023
3e8d7ae
add comments asking not to reformat
Feb 22, 2023
1cb6f7c
Update lib/assert.js
Feb 22, 2023
b7b8f64
Add a link to the issue in the added comments
Feb 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
assert: fix exception message for assert(0) on try catch block 
Fixes: #30872
  • Loading branch information
Hideki Nakamura committed Feb 22, 2023
commit 76fbda3e140f06a2cb1599df6a8e97d32b6d8290
44 changes: 24 additions & 20 deletions lib/assert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ let isDeepEqual;
let isDeepStrictEqual;
let parseExpressionAt;
let findNodeAround;
let tokenizer;
let decoder;

function lazyLoadComparison() {
Expand Down Expand Up @@ -242,39 +243,42 @@ function getCode(fd, line, column) {

function parseCode(code, offset) {
// Lazy load acorn.
if (parseExpressionAt === undefined) {
if (parseExpressionAt === undefined || tokenizer === undefined) {

@BridgeAR BridgeAR Feb 22, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (parseExpressionAt === undefined || tokenizer === undefined) {
if (parseExpressionAt === undefined) {

The second condition is not needed due to the tokenizer and parseExpressionAt being defined at the same moment.

@hidecology hidecology Feb 22, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BridgeAR
Thanks for your feedback.
I see. I will revert this conditional expression back to the previous one.

const Parser = require('internal/deps/acorn/acorn/dist/acorn').Parser;
({ findNodeAround } = require('internal/deps/acorn/acorn-walk/dist/walk'));

parseExpressionAt = FunctionPrototypeBind(Parser.parseExpressionAt, Parser);
tokenizer = FunctionPrototypeBind(Parser.tokenizer, Parser);
}
let node;
let start = 0;
let start;
// Parse the read code until the correct expression is found.
do {
for (const token of tokenizer(code, { ecmaVersion: 'latest' })) {
start = token.start;
if (start > offset) {
// No matching expression found. This could happen if the assert
// expression is bigger than the provided buffer.
break;
}
try {
node = parseExpressionAt(code, start, { ecmaVersion: 'latest' });
start = node.end + 1 || start;
// Find the CallExpression in the tree.
node = findNodeAround(node, offset, 'CallExpression');
} catch (err) {
// Unexpected token error and the like.
start += err.raisedAt || 1;
if (start > offset) {
// No matching expression found. This could happen if the assert
// expression is bigger than the provided buffer.
// eslint-disable-next-line no-throw-literal
throw null;
if (node && node.node.end >= offset) {

@BridgeAR BridgeAR Feb 22, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (node && node.node.end >= offset) {
if (node?.node.end >= offset) {

@hidecology hidecology Feb 22, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BridgeAR
Got it. I will change this code.

return [
node.node.start,
StringPrototypeReplace(StringPrototypeSlice(code,
node.node.start, node.node.end),
escapeSequencesRegExp, escapeFn),
];
}
// eslint-disable-next-line no-unused-vars
} catch (err) {
continue;
}
} while (node === undefined || node.node.end < offset);

return [
node.node.start,
StringPrototypeReplace(StringPrototypeSlice(code,
node.node.start, node.node.end),
escapeSequencesRegExp, escapeFn),
];
}
// eslint-disable-next-line no-throw-literal
throw null;
}

function getErrMessage(message, fn) {
Expand Down
43 changes: 43 additions & 0 deletions test/parallel/test-assert.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,49 @@ assert.throws(
'assert.ok(null)\n'
}
);
assert.throws(
() => {
try { assert.ok(0); // eslint-disable-line no-useless-catch, brace-style
Comment thread
BridgeAR marked this conversation as resolved.
} catch (err) {
throw err;
}
},
{
code: 'ERR_ASSERTION',
constructor: assert.AssertionError,
generatedMessage: true,
message: 'The expression evaluated to a falsy value:\n\n ' +
'assert.ok(0)\n'
}
);
assert.throws(
() => {
try {
throw new Error();
} catch (err) { assert.ok(0); } // eslint-disable-line no-unused-vars
},
{
code: 'ERR_ASSERTION',
constructor: assert.AssertionError,
generatedMessage: true,
message: 'The expression evaluated to a falsy value:\n\n ' +
'assert.ok(0)\n'
}
);
assert.throws(
() => {
function test() { assert.ok(0); // eslint-disable-line brace-style
}
test();
},
{
code: 'ERR_ASSERTION',
constructor: assert.AssertionError,
generatedMessage: true,
message: 'The expression evaluated to a falsy value:\n\n ' +
'assert.ok(0)\n'
}
);
assert.throws(
() => assert(typeof 123n === 'string'),
{
Expand Down