Skip to content

error: document removed error codes #22100

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

Closed
SirR4T wants to merge 9 commits into from
Closed

error: document removed error codes #22100

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0546c6a
[WIP] error: documenting removed error codes.
Aug 2, 2018
25a533f
doc: fix linter issues for legacy error codes
Aug 6, 2018
208cb4f
doc: Fix linter issues in errors.md
Aug 6, 2018
9b53c58
doc: add YAML parsers for removed sections
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
Next Next commit
[WIP] error: documenting removed error codes.
  • Loading branch information
@SirR4T
Sarat Addepalli authored and SirR4T committed Aug 23, 2018
commit 0546c6a79bb61d58e75dd4a5381929dd02fa2662
16 changes: 16 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,21 @@ 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="ERR_FS_WATCHER_ALREADY_STARTED"></a>
### ERR_FS_WATCHER_ALREADY_STARTED
<!-- YAML
added: v10.0.0
Copy link
Copy Markdown
Member

@BridgeAR BridgeAR Aug 6, 2018

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Author

@SirR4T SirR4T Aug 8, 2018

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
Copy Markdown
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
Copy Markdown
Contributor

@vsemozhetbyt vsemozhetbyt Aug 8, 2018

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
Copy Markdown
Author

@SirR4T SirR4T Aug 22, 2018

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
Copy Markdown
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
Copy Markdown
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.

-->
An attempt was made to start a watcher returned by `fs.watch()` that has already been started.
Copy link
Copy Markdown
Contributor

@vsemozhetbyt vsemozhetbyt Aug 3, 2018

Choose a reason for hiding this comment

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

Linter issue: 80 characters per line limit exceeded.


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

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

@vsemozhetbyt vsemozhetbyt Aug 3, 2018

Choose a reason for hiding this comment

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

Linter issue: 80 characters per line limit exceeded.


[`--force-fips`]: cli.html#cli_force_fips
[`'uncaughtException'`]: process.html#process_event_uncaughtexception
[`child_process`]: child_process.html
Expand Down Expand Up @@ -1890,6 +1905,7 @@ A module file could not be resolved while attempting a [`require()`][] or
[`zlib`]: zlib.html
[ES6 module]: esm.html
[Node.js Error Codes]: #nodejs-error-codes
[Legacy Node.js Error Codes]: #legacy-nodejs-error-codes
Copy link
Copy Markdown
Contributor

@vsemozhetbyt vsemozhetbyt Aug 3, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

  1. Linter issue: missing heading for this link?
  2. Doc tools use underscores, not hyphens in auto-created section ids, with document name prefix: #errors_legacy_node_js_error_codes for ## Legacy Node.js Error Codes.

Copy link
Copy Markdown
Author

@SirR4T SirR4T Aug 6, 2018

Choose a reason for hiding this comment

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

doh! missed adding the h2 header, fixing it now. Also, realized that I don't really need the link definition at the end of the file, so removed it.

[V8's stack trace API]: https://github.com/v8/v8/wiki/Stack-Trace-API
[WHATWG URL API]: url.html#url_the_whatwg_url_api
[crypto digest algorithm]: crypto.html#crypto_crypto_gethashes
Expand Down