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

doc: update System Errors documentation #24090

Closed
wants to merge 6 commits into from

Conversation

Trott
Copy link
Member

@Trott Trott commented Nov 4, 2018

Simplify text. Add explanation that code is the most stable way to
identify an error, in contrast with message which is subject to change
between patch-level versions of Node.js. Synchronize list of properties
with text. Order properties alphabetically.

Fixes: #23975

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. errors Issues and PRs related to JavaScript errors originated in Node.js core. labels Nov 4, 2018
doc/api/errors.md Outdated Show resolved Hide resolved
doc/api/errors.md Outdated Show resolved Hide resolved
doc/api/errors.md Outdated Show resolved Hide resolved
doc/api/errors.md Outdated Show resolved Hide resolved
Copy link
Member

@joyeecheung joyeecheung left a comment

Choose a reason for hiding this comment

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

Thank you!

doc/api/errors.md Outdated Show resolved Hide resolved
doc/api/errors.md Show resolved Hide resolved
@joyeecheung
Copy link
Member

Oh, wait...I don't think this fix #23975 because we not only attach codes to SystemErrors, but also a bunch of TypeErrors and such - and in fact only 3 of our coded errors are SystemErrors. Ideally there should be a section front and center that states what's currently in the error.code section of SystemError to properly fix that issue..

doc/api/errors.md Outdated Show resolved Hide resolved
doc/api/errors.md Outdated Show resolved Hide resolved
doc/api/errors.md Outdated Show resolved Hide resolved
doc/api/errors.md Outdated Show resolved Hide resolved
doc/api/errors.md Outdated Show resolved Hide resolved
Simplify text. Add explanation that `code` is the most stable way to
identify an error, in contrast with `message` which is subject to change
between patch-level versions of Node.js. Synchronize list of properties
with text. Order properties alphabetically.
@Trott
Copy link
Member Author

Trott commented Nov 6, 2018

Ideally there should be a section front and center

I've moved the note about error.code above out of the System Errors section. I think that should do it, at least minimally?

@Trott
Copy link
Member Author

Trott commented Nov 6, 2018

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 6, 2018
@Trott
Copy link
Member Author

Trott commented Nov 7, 2018

Landed in 18a4550...1368eb8

@Trott Trott closed this Nov 7, 2018
Trott added a commit to Trott/io.js that referenced this pull request Nov 7, 2018
Simplify text. Add explanation that `code` is the most stable way to
identify an error, in contrast with `message` which is subject to change
between patch-level versions of Node.js. Synchronize list of properties
with text. Order properties alphabetically.

PR-URL: nodejs#24090
Fixes: nodejs#23975
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
Trott added a commit to Trott/io.js that referenced this pull request Nov 7, 2018
Fixes: nodejs#23975

PR-URL: nodejs#24090
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@cjihrig cjihrig mentioned this pull request Nov 7, 2018
3 tasks
BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
Simplify text. Add explanation that `code` is the most stable way to
identify an error, in contrast with `message` which is subject to change
between patch-level versions of Node.js. Synchronize list of properties
with text. Order properties alphabetically.

PR-URL: #24090
Fixes: #23975
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
BridgeAR pushed a commit that referenced this pull request Nov 14, 2018
Fixes: #23975

PR-URL: #24090
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
kiyomizumia pushed a commit to kiyomizumia/node that referenced this pull request Nov 15, 2018
Simplify text. Add explanation that `code` is the most stable way to
identify an error, in contrast with `message` which is subject to change
between patch-level versions of Node.js. Synchronize list of properties
with text. Order properties alphabetically.

PR-URL: nodejs#24090
Fixes: nodejs#23975
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
kiyomizumia pushed a commit to kiyomizumia/node that referenced this pull request Nov 15, 2018
Fixes: nodejs#23975

PR-URL: nodejs#24090
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
codebytere pushed a commit that referenced this pull request Nov 29, 2018
Simplify text. Add explanation that `code` is the most stable way to
identify an error, in contrast with `message` which is subject to change
between patch-level versions of Node.js. Synchronize list of properties
with text. Order properties alphabetically.

PR-URL: #24090
Fixes: #23975
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
codebytere pushed a commit that referenced this pull request Nov 29, 2018
Fixes: #23975

PR-URL: #24090
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 29, 2018
Simplify text. Add explanation that `code` is the most stable way to
identify an error, in contrast with `message` which is subject to change
between patch-level versions of Node.js. Synchronize list of properties
with text. Order properties alphabetically.

PR-URL: #24090
Fixes: #23975
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Nov 29, 2018
Fixes: #23975

PR-URL: #24090
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@codebytere codebytere mentioned this pull request Nov 29, 2018
MylesBorins pushed a commit that referenced this pull request Dec 3, 2018
Simplify text. Add explanation that `code` is the most stable way to
identify an error, in contrast with `message` which is subject to change
between patch-level versions of Node.js. Synchronize list of properties
with text. Order properties alphabetically.

PR-URL: #24090
Fixes: #23975
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Dec 3, 2018
Fixes: #23975

PR-URL: #24090
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>
@Trott Trott deleted the edit-system-errors branch January 13, 2022 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. doc Issues and PRs related to the documentations. errors Issues and PRs related to JavaScript errors originated in Node.js core.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

doc: is the stability guarantee of err.code documented anywhere?
6 participants