Skip to content

errors: improve ERR_INVALID_OPT_VALUE message - #34671

Closed
lundibundi wants to merge 2 commits into
nodejs:masterfrom
lundibundi:improve-errors-opt-value
Closed

errors: improve ERR_INVALID_OPT_VALUE message#34671
lundibundi wants to merge 2 commits into
nodejs:masterfrom
lundibundi:improve-errors-opt-value

Conversation

@lundibundi

@lundibundilundibundi commented Aug 7, 2020

Copy link
Copy Markdown
Member
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

I also thought of changing the message from always having the quotes (i.e. "42" -> 42) around value to make it easier to distinguish between different value types (i.e. string vs number), wdyt?

@lundibundi
errors: improve ERR_INVALID_OPT_VALUE error
d87d4d0
* use util.inspect for value presentation
* allow to optionally specify error reason
@nodejs-github-botnodejs-github-bot added child_process Issues and PRs related to the child_process subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. net Issues and PRs related to the net subsystem. labels Aug 7, 2020
@nodejs-github-bot

nodejs-github-bot commented Aug 7, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32678/

@lundibundilundibundi mentioned this pull request Aug 7, 2020
Closed
3 tasks
Trott
Trott reviewed Aug 9, 2020
View reviewed changes
Comment threadlib/internal/errors.js Outdated
Trott
Trott reviewed Aug 9, 2020
View reviewed changes
Comment threadtest/parallel/test-crypto-keygen.js Outdated
Trott
Trott approved these changes Aug 9, 2020
View reviewed changes

@TrottTrott left a comment

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.

Left two optional nits, but looks good to me with or without them.

@lundibundi
lundibundi requested a review from a team as a code ownerAugust 10, 2020 16:06
@lundibundi
lundibundi requested a review from a teamAugust 10, 2020 16:06
@lundibundilundibundi added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 14, 2020
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 14, 2020
@nodejs-github-bot

nodejs-github-bot commented Aug 14, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/32788/

@lundibundilundibundi added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 14, 2020
mmarchini pushed a commit that referenced this pull request Aug 15, 2020
@lundibundi@mmarchini
errors: improve ERR_INVALID_OPT_VALUE error
8da8ec9
* use util.inspect for value presentation
* allow to optionally specify error reason
PR-URL: #34671
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mary Marchini <oss@mmarchini.me>
@mmarchini

mmarchini commented Aug 15, 2020

Copy link
Copy Markdown
Contributor

Landed in 8da8ec9

MylesBorins pushed a commit that referenced this pull request Aug 17, 2020
@lundibundi@MylesBorins
errors: improve ERR_INVALID_OPT_VALUE error
cbf53ec
* use util.inspect for value presentation
* allow to optionally specify error reason
PR-URL: #34671
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mary Marchini <oss@mmarchini.me>
@danielleadamsdanielleadams mentioned this pull request Aug 20, 2020
Merged
BethGriggs pushed a commit that referenced this pull request Aug 20, 2020
@lundibundi@BethGriggs
errors: improve ERR_INVALID_OPT_VALUE error
dbcb36d
* use util.inspect for value presentation
* allow to optionally specify error reason
PR-URL: #34671
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mary Marchini <oss@mmarchini.me>
MylesBorins pushed a commit that referenced this pull request Nov 3, 2020
@lundibundi@MylesBorins
errors: improve ERR_INVALID_OPT_VALUE error
3179fa5
* use util.inspect for value presentation
* allow to optionally specify error reason
PR-URL: #34671
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mary Marchini <oss@mmarchini.me>
@MylesBorinsMylesBorins mentioned this pull request Nov 3, 2020
Merged
MylesBorins pushed a commit that referenced this pull request Nov 16, 2020
@lundibundi@MylesBorins
errors: improve ERR_INVALID_OPT_VALUE error
f22672d
* use util.inspect for value presentation
* allow to optionally specify error reason
PR-URL: #34671
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Mary Marchini <oss@mmarchini.me>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TrottTrottTrott approved these changes
@jasnelljasnelljasnell approved these changes
+1 more reviewer
@mmarchinimmarchinimmarchini approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.child_processIssues and PRs related to the child_process subsystem.errorsIssues and PRs related to JavaScript errors originated in Node.js core.netIssues and PRs related to the net subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@lundibundi@nodejs-github-bot@mmarchini@jasnell@Trott