Skip to content

Conversation

@joyeecheung
Copy link
Member

@joyeecheungjoyeecheung commented Jan 24, 2018

  • Allow user to customize why the argument is invalid
  • Display the argument with util.inspect so null bytes can be
    displayed properly.

Spinning off from #18308 , but I think this can be submitted alone since that one needs a bit more reviews to land and that's semver-major. The current formatter does not allow users to explain why the argument is invalid and it displays the argument with ${String(value)} which cannot display null bytes properly. This patch makes the error message more debuggable.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

errors

- Allow user to customize why the argument is invalid - Display the argument with util.inspect so null bytes can be displayed properly.
@nodejs-github-botnodejs-github-bot added the errors Issues and PRs related to JavaScript errors originated in Node.js core. label Jan 24, 2018
@joyeecheung
Copy link
MemberAuthor

@joyeecheung
Copy link
MemberAuthor

CI failures look unrelated.

@joyeecheungjoyeecheung added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 25, 2018
@joyeecheung
Copy link
MemberAuthor

Landed in 3ec7921, thanks!

@joyeecheungjoyeecheung removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 29, 2018
joyeecheung added a commit that referenced this pull request Jan 29, 2018
- Allow user to customize why the argument is invalid - Display the argument with util.inspect so null bytes can be displayed properly. PR-URL: #18358 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jon Moss <[email protected]>
@MylesBorins
Copy link
Contributor

This does not land cleanly on v9.x, would it be a good idea the backport?

joyeecheung added a commit to joyeecheung/node that referenced this pull request Feb 22, 2018
- Allow user to customize why the argument is invalid - Display the argument with util.inspect so null bytes can be displayed properly. PR-URL: nodejs#18358 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jon Moss <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 26, 2018
- Allow user to customize why the argument is invalid - Display the argument with util.inspect so null bytes can be displayed properly. Backport-PR-URL: #18916 PR-URL: #18358 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jon Moss <[email protected]>
MylesBorins pushed a commit that referenced this pull request Feb 26, 2018
- Allow user to customize why the argument is invalid - Display the argument with util.inspect so null bytes can be displayed properly. Backport-PR-URL: #18916 PR-URL: #18358 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jon Moss <[email protected]>
@addaleaxaddaleax mentioned this pull request Feb 27, 2018
joyeecheung added a commit to joyeecheung/node that referenced this pull request May 2, 2018
PR-URL: nodejs#18358 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jon Moss <[email protected]>
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
- Allow user to customize why the argument is invalid - Display the argument with util.inspect so null bytes can be displayed properly. PR-URL: nodejs#18358 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jon Moss <[email protected]>
MylesBorins pushed a commit that referenced this pull request May 22, 2018
Backport-PR-URL: #19191 PR-URL: #18358 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jon Moss <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jun 14, 2018
Backport-PR-URL: #19191 PR-URL: #18358 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jon Moss <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Jul 9, 2018
@codebytere
Copy link
Member

@joyeecheung this doesn't apply cleanly to v8.x either, should it be backported?

rvagg pushed a commit that referenced this pull request Aug 16, 2018
Backport-PR-URL: #19191 PR-URL: #18358 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jon Moss <[email protected]>
@jasnell
Copy link
Member

I don't believe this one should be backported to 8.x

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

errorsIssues and PRs related to JavaScript errors originated in Node.js core.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@joyeecheung@MylesBorins@codebytere@jasnell@maclover7@nodejs-github-bot