-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
lib/child_process: convert to using internal/errors #15090
Conversation
lib/child_process.js
Outdated
@@ -390,47 +391,47 @@ function normalizeSpawnArguments(file, args, options) { | |||
if (options === undefined) | |||
options = {}; | |||
else if (options === null || typeof options !== 'object') | |||
throw new TypeError('"options" argument must be an object'); | |||
throw new errors.TypeError('ERR_OPTS_NOT_OBJECT'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The existing ERR_INVALID_ARG_TYPE
would be better here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(and else where throughout here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for doing this. I may be mistaken but I seem to recall another open PR that was looking at this file also. I'd recommend doing a quick search to make sure you're not accidentally duplicating effort. Either way, the work is deeply appreciated.
I left a couple of comments that should be addressed before this could proceed forward.
The other PR is here: #14998 |
They both change errors in |
so can i close this issue? |
Closing as this is a duplicate of another PR that is about to land. |
@ramimoshe thanks a lot for your work nevertheless! I am sorry that your PR could not land. |
covert lib/child_process.js over to using lib/internal/errors.js
ref: #11273