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.

Sign up for GitHub

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

error: document removed error codes #22100

Closed
wants to merge 9 commits into from
Closed

error: document removed error codes #22100

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0546c6a
[WIP] error: documenting removed error codes.
SirR4T Aug 2, 2018
25a533f
doc: fix linter issues for legacy error codes
SirR4T Aug 6, 2018
208cb4f
doc: Fix linter issues in errors.md
SirR4T Aug 6, 2018
9b53c58
doc: add YAML parsers for removed sections
SirR4T Aug 7, 2018
dd31ede
doc: update list of deprecated / removed error codes
SirR4T Aug 23, 2018
1ad00f2
docs: fix typos in error docs
SirR4T Aug 24, 2018
e77d298
doc: errors that have been removed but not released
SirR4T Aug 24, 2018
e536153
error: reorder errors which were never released
SirR4T Aug 25, 2018
f10d321
doc: removed hard coded handles where unnecessary
SirR4T Aug 26, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
245 changes: 245 additions & 0 deletions doc/api/errors.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1848,6 +1848,250 @@ Creation of a [`zlib`][] object failed due to incorrect configuration.
A module file could not be resolved while attempting a [`require()`][] or
`import` operation.

<a id="legacy-nodejs-error-codes"></a>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I may be wrong, but we may need not this hardcoded id, as the other ones in this doc may be due to legacy links support. Let us see what others think.

## Legacy Node.js Error Codes

> Stability: 0 - Deprecated. These error codes are either inconsistent, or have
> been removed.

<a id="ERR_FS_WATCHER_ALREADY_STARTED"></a>
### ERR_FS_WATCHER_ALREADY_STARTED
<!-- YAML
added: v10.0.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if we add a "removed: v10.x.x". I guess that might not be parsed properly tough. @vsemozhetbyt do you have a suggestion for this?

Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Aug 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may add the case for removing to adding and deprecating. It seems these two function need to be updated accordingly:

node/tools/doc/common.js

Line 14 in 2bea9ce

function extractAndParseYAML(text) {

node/tools/doc/html.js

Line 258 in 2bea9ce

function parseYAML(text) {

(Just grepping 'added' or 'deprecated' and adding a similar case may do)

Copy link
Contributor Author

@SirR4T SirR4T Aug 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BridgeAR added these sections (thanks @vsemozhetbyt, for the pointers), now shows up as
screen shot 2018-08-07 at 10 34 04 am

Although cc: @joyeecheung , because it seems to me she has some reservations against saying "removed in v10.x.x" .

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, should there be a short explanation on why this error code was removed? Something on the lines of

Like fs.watchFile(), this situation is now silently ignored.

or some such.

Copy link
Member

@joyeecheung joyeecheung Aug 8, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SirR4T I think it's OK to have removed in, but the version number should be accurate. In the case of ERR_FS_WTCHER_ALREADY_STARTED, it has never been released in any version, since the commit that added it (6c25f2e) and the commit that removed it (301f6cc) are both present from v10.0.0...v10.8.0 - which means when v10.0.0 was out, the error was not even there. Therefore, this error code has only appeared in nightly and canary releases.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do add a short explanation, we can make this in changes blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it. If the error codes never actually touched the releases, what should the description look like? is having both added: v10.0.0 and removed: v10.0.0 ok?

Copy link
Contributor Author

@SirR4T SirR4T Aug 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joyeecheung : also, I have added versions added and versions removed here, for all the commits which either added or removed the error codes. Eyeballing the versions, it seems to me that the wherever github mentions vA.a.a .. vB.b.b, we should be mentioning the vB.b.b in the docs. (For both added, as well as removed.) will that be correct?

Copy link
Member

@joyeecheung joyeecheung Aug 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think annotating them as added: v10.0.0 removed: v10.0.0 is more confusing than helpful. A note in the docs about that would be enough, maybe something like:

This error code has never been released and only existed in nightly builds.

removed: v10.0.0
-->
An attempt was made to start a watcher returned by `fs.watch()` that has
already been started.

<a id="ERR_FS_WATCHER_NOT_STARTED"></a>
### ERR_FS_WATCHER_NOT_STARTED
<!-- YAML
added: v10.0.0
removed: v10.0.0
-->

An attempt was made to initiate operations on a watcher returned by
`fs.watch()` that has not yet been started.

<a id="ERR_HTTP2_ALREADY_SHUTDOWN"></a>
### ERR_HTTP2_ALREADY_SHUTDOWN
<!-- YAML
added: v10.0.0
removed: v10.0.0
-->

Occurs with multiple attempts to shutdown an HTTP/2 session.

<a id="ERR_HTTP2_ERROR"></a>
### ERR_HTTP2_ERROR
<!-- YAML
added: v9.0.0
removed: v9.0.0
-->

A non-specific HTTP/2 error has occurred.

<a id="ERR_HTTP2_FRAME_ERROR"></a>
### ERR_HTTP2_FRAME_ERROR
<!-- YAML
added: v9.0.0
removed: v10.0.0
-->

Used when a failure occurs sending an individual frame on the HTTP/2
session.

<a id="ERR_HTTP2_HEADERS_OBJECT"></a>
### ERR_HTTP2_HEADERS_OBJECT
<!-- YAML
added: v9.0.0
removed: v10.0.0
-->

Used when an HTTP/2 Headers Object is expected.

<a id="ERR_HTTP2_HEADER_REQUIRED"></a>
### ERR_HTTP2_HEADER_REQUIRED
<!-- YAML
added: v9.0.0
removed: v10.0.0
-->

Used when a required header is missing in an HTTP/2 message.

<a id="ERR_HTTP2_INFO_HEADERS_AFTER_RESPOND"></a>
### ERR_HTTP2_INFO_HEADERS_AFTER_RESPOND
<!-- YAML
added: v9.0.0
removed: v10.0.0
-->

HTTP/2 Informational headers must only be sent *prior* to calling the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Informational -> informational (lower case).

`Http2Stream.prototype.respond()` method.

<a id="ERR_HTTP2_STREAM_CLOSED"></a>
### ERR_HTTP2_STREAM_CLOSED
<!-- YAML
added: v9.0.0
removed: v10.0.0
-->

Used when an action has been performed on an HTTP/2 Stream that has already
been closed.

<a id="ERR_HTTP_INVALID_CHAR"></a>
### ERR_HTTP_INVALID_CHAR
<!-- YAML
added: v9.0.0
removed: v10.0.0
-->

Used when an invalid character is found in an HTTP response status message
(reason phrase).

<a id="ERR_INVALID_ARRAY_LENGTH"></a>
### ERR_INVALID_ARRAY_LENGTH
<!-- YAML
added: v9.0.0
removed: REPLACEME
-->

Used when an Array is not of the expected length or in a valid range.

<a id="ERR_INVALID_DOMAIN_NAME"></a>
### ERR_INVALID_DOMAIN_NAME
<!-- YAML
added: v9.0.0
removed: REPLACEME
-->

Used when `hostname` can not be parsed from a provided URL.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can not -> cannot


<a id="ERR_INVALID_REPL_HISTORY"></a>
### ERR_INVALID_REPL_HISTORY
<!-- YAML
added: v9.0.0
removed: v9.0.0
-->

Used in the `repl` in case the old history file is used and an error occurred
while trying to read and parse it.

<a id="ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK"></a>
### ERR_MISSING_DYNAMIC_INSTANTIATE_HOOK
<!-- YAML
added: v10.0.0
removed: v10.0.0
-->

Used when an [ES6 module][] loader hook specifies `format: 'dynamic'` but does
not provide a `dynamicInstantiate` hook.

<a id="ERR_NAPI_CONS_PROTOTYPE_OBJECT"></a>
### ERR_NAPI_CONS_PROTOTYPE_OBJECT
<!-- YAML
added: v9.0.0
removed: v10.0.0
-->

Used by the `N-API` when `Constructor.prototype` is not an object.

<a id="ERR_OUTOFMEMORY"></a>
### ERR_OUTOFMEMORY
<!-- YAML
added: v9.0.0
removed: v10.0.0
-->

Used generically to identify that an operation caused an out of memory
condition.

<a id="ERR_PARSE_HISTORY_DATA"></a>
### ERR_PARSE_HISTORY_DATA
<!-- YAML
added: v9.0.0
removed: v10.0.0
-->

The `repl` module was unable parse data from the REPL history file.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unable parse -> unable to parse


<a id="ERR_STREAM_HAS_STRINGDECODER"></a>
### ERR_STREAM_HAS_STRINGDECODER
<!-- YAML
added: v9.0.0
removed: v9.0.0
-->

Used to prevent an abort if a string decoder was set on the Socket.

Example
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe omit the word Example as obvious.

```js
const Socket = require('net').Socket;
const instance = new Socket();

instance.setEncoding('utf-8');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'utf-8' -> 'utf8'

```

<a id="ERR_STREAM_READ_NOT_IMPLEMENTED"></a>
### ERR_STREAM_READ_NOT_IMPLEMENTED
<!-- YAML
added: v9.0.0
removed: v10.0.0
-->

Used when an attempt is made to use a readable stream that has not implemented
[`readable._read()`][].

<a id="ERR_STRING_TOO_LARGE"></a>
### ERR_STRING_TOO_LARGE
<!-- YAML
added: v10.0.0
removed: v10.0.0
-->

An attempt has been made to create a string larger than the maximum allowed
size.

<a id="ERR_TLS_RENEGOTIATION_FAILED"></a>
### ERR_TLS_RENEGOTIATION_FAILED
<!-- YAML
added: v9.0.0
removed: v10.0.0
-->

Used when a TLS renegotiation request has failed in a non-specific way.

<a id="ERR_UNKNOWN_BUILTIN_MODULE"></a>
### ERR_UNKNOWN_BUILTIN_MODULE
<!-- YAML
added: v8.0.0
removed: v9.0.0
-->

The `'ERR_UNKNOWN_BUILTIN_MODULE'` error code is used to identify a specific
kind of internal Node.js error that should not typically be triggered by user
code. Instances of this error point to an internal bug within the Node.js
binary itself.

<a id="ERR_VALUE_OUT_OF_RANGE"></a>
### ERR_VALUE_OUT_OF_RANGE
<!-- YAML
added: v9.0.0
removed: v10.0.0
-->

Used when a given value is out of the accepted range.

<a id="ERR_ZLIB_BINDING_CLOSED"></a>
### ERR_ZLIB_BINDING_CLOSED
<!-- YAML
added: v9.0.0
removed: v10.0.0
-->

Used when an attempt is made to use a `zlib` object after it has already been
closed.

[`--force-fips`]: cli.html#cli_force_fips
[`'uncaughtException'`]: process.html#process_event_uncaughtexception
[`child_process`]: child_process.html
Expand Down Expand Up @@ -1875,6 +2119,7 @@ A module file could not be resolved while attempting a [`require()`][] or
[`new URLSearchParams(iterable)`]: url.html#url_constructor_new_urlsearchparams_iterable
[`process.send()`]: process.html#process_process_send_message_sendhandle_options_callback
[`process.setUncaughtExceptionCaptureCallback()`]: process.html#process_process_setuncaughtexceptioncapturecallback_fn
[`readable._read()`]: stream.html#stream_readable_read_size_1
[`require()`]: modules.html#modules_require
[`require('crypto').setEngine()`]: crypto.html#crypto_crypto_setengine_engine_flags
[`server.listen()`]: net.html#net_server_listen
Expand Down
4 changes: 4 additions & 0 deletions tools/doc/common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ function extractAndParseYAML(text) {
meta.deprecated = arrify(meta.deprecated);
}

if (meta.removed) {
meta.removed = arrify(meta.removed);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed for this PR? From what I can tell we only used single removed fields here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was blindly copy pasting wherever added occurred.

Maybe not for this particular PR, but if we're planning to add removed as a feature for api docs, would rather it supports semver-minors as well, like added and deprecated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SirR4T Ah, sorry, I thought we already had removed before, looks like it is new in this PR...

}

meta.changes = meta.changes || [];

return meta;
Expand Down
10 changes: 9 additions & 1 deletion tools/doc/html.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ function parseYAML(text) {

const added = { description: '' };
const deprecated = { description: '' };
const removed = { description: '' };

if (meta.added) {
added.version = meta.added.join(', ');
Expand All @@ -276,9 +277,15 @@ function parseYAML(text) {
`<span>Deprecated since: ${deprecated.version}</span>`;
}

if (meta.removed) {
removed.version = meta.removed.join(', ');
removed.description = `<span>Removed in: ${removed.version}</span>`;
}

if (meta.changes.length > 0) {
if (added.description) meta.changes.push(added);
if (deprecated.description) meta.changes.push(deprecated);
if (removed.description) meta.changes.push(removed);

meta.changes.sort((a, b) => versionSort(a.version, b.version));

Expand All @@ -299,7 +306,8 @@ function parseYAML(text) {

result += '</table>\n</details>\n';
} else {
result += `${added.description}${deprecated.description}\n`;
result += `${added.description}${deprecated.description}` +
`${removed.description}\n`;
}

if (meta.napiVersion) {
Expand Down