★ wanayoo — archive 1999 https://github.com/nodejs/node/pull/18358Nouvelle recherche | Portail wanayoo
Skip to content
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.

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

errors: improve the description of ERR_INVALID_ARG_VALUE #18358

Closed
wants to merge 1 commit into from

Conversation

joyeecheung
Copy link
Member

@joyeecheung joyeecheung 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.
@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung commented Jan 24, 2018

@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung commented Jan 25, 2018

CI failures look unrelated.

@joyeecheung
Copy link
Member Author

@joyeecheung joyeecheung commented Jan 29, 2018

Landed in 3ec7921, thanks!

joyeecheung added a commit that referenced this issue 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 <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
@MylesBorins
Copy link
Member

@MylesBorins MylesBorins commented Feb 20, 2018

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 issue 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 <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins added a commit that referenced this issue 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 <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins added a commit that referenced this issue 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 <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
@addaleax addaleax mentioned this pull request Feb 27, 2018
joyeecheung added a commit to joyeecheung/node that referenced this issue May 2, 2018
PR-URL: nodejs#18358
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MayaLekova added a commit to MayaLekova/node that referenced this issue 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 <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins added a commit that referenced this issue May 22, 2018
Backport-PR-URL: #19191
PR-URL: #18358
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
MylesBorins added a commit that referenced this issue Jun 14, 2018
Backport-PR-URL: #19191
PR-URL: #18358
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
@MylesBorins MylesBorins mentioned this pull request Jul 9, 2018
@codebytere
Copy link
Member

@codebytere codebytere commented Aug 2, 2018

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

rvagg added a commit that referenced this issue Aug 16, 2018
Backport-PR-URL: #19191
PR-URL: #18358
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
@jasnell
Copy link
Member

@jasnell jasnell commented Aug 17, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants