Skip to content

errors: validate input arguments #19924

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

Closed
BridgeAR wants to merge 2 commits into from
Closed

errors: validate input arguments #19924

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
76a8ce5
errors: validate input arguments
BridgeAR Apr 11, 2018
edf2852
fixup: add comments
BridgeAR Apr 12, 2018
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
Prev Previous commit
fixup: add comments
  • Loading branch information
@BridgeAR
BridgeAR committed Apr 12, 2018
commit edf2852801b75ec16a22b8cd626cd0233ae03755
6 changes: 6 additions & 0 deletions lib/internal/errors.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -934,6 +934,8 @@ E('ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET',
Error);
E('ERR_UNESCAPED_CHARACTERS', '%s contains unescaped characters', TypeError);
E('ERR_UNHANDLED_ERROR',
// Using a default argument here is important so the argument is not counted
// towards `Function#length`.
(err = undefined) => {
Copy link
Member

@mcollina mcollina Apr 12, 2018

Choose a reason for hiding this comment

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

I don't think this is truly needed. Or is it?

Copy link
Member Author

@BridgeAR BridgeAR Apr 12, 2018

Choose a reason for hiding this comment

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

Sadly that is necessary. Default arguments do not count towards function.length as in:

function a(a) {}
function b(a = undefined) {}
assert.strictEqual(a.length, 1);
assert.strictEqual(b.length, 0);

Otherwise it is difficult to validate a couple of functions, but I think it is worth it because it makes sure we make less errors while implementing the errors.

Copy link
Member

@mcollina mcollina Apr 12, 2018

Choose a reason for hiding this comment

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

Can you add a comment before the function then? Otherwise it won't be clear to the reader.

Copy link
Member Author

@BridgeAR BridgeAR Apr 12, 2018

Choose a reason for hiding this comment

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

Done.

const msg = 'Unhandled error.';
if (err === undefined) return msg;
Expand Down Expand Up @@ -1034,13 +1036,17 @@ function oneOf(expected, thing) {
}
}

// Using a default argument here is important so the argument is not counted
// towards `Function#length`.
function bufferOutOfBounds(name = undefined) {
Copy link
Member

@mcollina mcollina Apr 12, 2018

Choose a reason for hiding this comment

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

I don't think the default argument is truly needed.

Copy link
Member Author

@BridgeAR BridgeAR Apr 12, 2018

Choose a reason for hiding this comment

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

See above.

if (name) {
return `"${name}" is outside of buffer bounds`;
}
return 'Attempt to write outside buffer bounds';
}

// Using a default argument here is important so the argument is not counted
// towards `Function#length`.
function invalidChar(name, field = undefined) {
let msg = `Invalid character in ${name}`;
if (field !== undefined) {
Expand Down