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: similar error codes [VALUE_OUT_OF_RANGE] and [OUT_OF_RANGE] #17603

Closed
starkwang opened this issue Dec 11, 2017 · 3 comments
Closed

errors: similar error codes [VALUE_OUT_OF_RANGE] and [OUT_OF_RANGE] #17603

starkwang opened this issue Dec 11, 2017 · 3 comments
Labels
errors Issues and PRs related to JavaScript errors originated in Node.js core.

Comments

@starkwang
Copy link
Contributor

starkwang commented Dec 11, 2017

Now there are two similar error codes in lib:

Should we reduce them into one?

IMHO, the error message of ERR_VALUE_OUT_OF_RANGE is more detailed instead of a simple string('The "%s" argument is out of range') of ERR_OUT_OF_RANGE, so we might be able to migrate the ERR_OUT_OF_RANGE into ERR_VALUE_OUT_OF_RANGE.

  • Version: nodejs/node:master
  • Platform: all
  • Subsystem: errors
@Fishrock123
Copy link
Contributor

cc @jasnell

@BridgeAR
Copy link
Member

I am a fan of combining such similar error types. So +1 from me. I would say, just go ahead and open a PR.

@benjamingr
Copy link
Member

+1 for me as well, semver-major though.

@addaleax addaleax added the errors Issues and PRs related to JavaScript errors originated in Node.js core. label Dec 13, 2017
starkwang added a commit to starkwang/node that referenced this issue Dec 23, 2017
There two similar error codes in lib: "ERR_VALUE_OUT_OF_RANGE"
and "ERR_OUT_OF_RANGE". This change is to reduce them into
"ERR_VALUE_OUT_OF_RANGE"

Fixes: nodejs#17603
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
errors Issues and PRs related to JavaScript errors originated in Node.js core.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants