-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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, console: migrate to use internal/errors.js #11308
Conversation
@@ -575,6 +629,7 @@ found [here][online]. | |||
[domains]: domain.html | |||
[event emitter-based]: events.html#events_class_eventemitter | |||
[file descriptors]: https://en.wikipedia.org/wiki/File_descriptor | |||
[Node.js Error Codes]: #nodejs-error-codes |
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.
I think it will be _
not -
. You can confirm this by building the docs locally, with make docopen
.
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.
I just checked and #nodejs-error-codes
works.
Related: #11319 also has an implementation for |
Yes, some duplication across these is fine. We'll sort it out by rebasing as things get landed. |
|
||
function Console(stdout, stderr) { | ||
if (!(this instanceof Console)) { | ||
return new Console(stdout, stderr); | ||
} | ||
if (!stdout || typeof stdout.write !== 'function') { | ||
throw new TypeError('Console expects a writable stream instance'); | ||
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'stdout', 'WriteStream'); |
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.
Hmm, wait, WriteStream
is not an actual type or class name (at least the constructor of writable stream from our stream
module is named Writable
). Is it OK to pass a descriptive type (like "writable stream") to the formatter of ERR_INVALID_ARG_TYPE
? @jasnell
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.
Oh right, there are tty.WriteStream
and fs.WriteStream
, but then those subclass Writable
and the condition here even only checks .write
, not instanceof
.
I'm closing this because it's been inactive for quite a while. Feel free to reopen or ping a collaborator to get it reopened if needed. |
Migrate lib/console.js to use internal/errors.js.
Refs: #11273
cc @jasnell
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)
errors, console