doc: make error descriptions more concise #16954

Trott · 2017-11-11T15:44:46Z

Remove the practice of starting most error descriptions with "Used when"
or wordier variations.

Change errors of the form:

Used when the type of an asynchronous resource is invalid.

...to:

The type of an asynchronous resource was invalid.

Change errors of the form:

The 'ERR_INVALID_CURSOR_POS' is thrown specifically when a cursor on
a given stream is attempted to move to a specified row without a specified
column.

...to:

A cursor on a given stream cannot be moved to a specified row without
a specified column.

Checklist

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

Affected core subsystem(s)

errors doc

maclover7

left a few comments, some are just adding a comma here or there, feel free to treat as nits :)

maclover7 · 2017-11-11T22:54:13Z

doc/api/errors.md


 <a id="ERR_CPU_USAGE"></a>
 ### ERR_CPU_USAGE

-Used when the native call from `process.cpuUsage` cannot be processed properly.
+The native call from `process.cpuUsage` could be processed properly.


could --> could not?

Done. I also removed properly as it seems superfluous.

maclover7 · 2017-11-11T22:55:03Z

doc/api/errors.md


 <a id="ERR_CRYPTO_FIPS_UNAVAILABLE"></a>
 ### ERR_CRYPTO_FIPS_UNAVAILABLE

-Used when trying to enable or disable FIPS mode when FIPS is not available.
+There was an attempt to enable or disable FIPS mode but FIPS mode is not


comma after disable FIPS mode?

done, thanks

maclover7 · 2017-11-11T22:57:31Z

doc/api/errors.md


 <a id="ERR_HTTP2_HEADER_SINGLE_VALUE"></a>
 ### ERR_HTTP2_HEADER_SINGLE_VALUE

-Used when multiple values have been provided for an HTTP header field that
-required to have only a single value.
+Multiple values have been provided for an HTTP header field that is required to


I know it wasn't there originally, but should HTTP be changes to HTTP/2?

It would seem so. Done.

maclover7 · 2017-11-11T22:58:41Z

doc/api/errors.md


 <a id="ERR_HTTP2_PUSH_DISABLED"></a>
 ### ERR_HTTP2_PUSH_DISABLED

-Used when push streams have been disabled by the client but an attempt to
-create a push stream is made.
+Push streams have been disabled by the client but an attempt to create a push


comma after by the client?

done, thanks

jasnell

LGTM. FWIW, once the error conversion is complete, I'm planning a sprint of changes to fill in the details on these errors in the docs, including example code (where possible) showing how to trigger the error and an explanation on how to fix it.

benjamingr

Making @maclover7 's nit explicit - otherwise looks like a nice positive change

Trott · 2017-11-12T14:03:48Z

@benjamingr I've addressed all of @maclover7's nits and also edited a few more messages that landed since I originally opened this. PTAL. (I left these changes in separate commits for easier reviewing if you don't want to read everything all over again.)

maclover7 · 2017-11-12T14:10:18Z

LGTM

benjamingr · 2017-11-12T15:00:16Z

@Trott just in case you haven't noticed I've approved those changes (after your commit but before your comment).

Trott · 2017-11-14T19:22:07Z

CI: https://ci.nodejs.org/job/node-test-pull-request/11435/

joyeecheung

Left a bunch of suggestions...for better consistency 😅

joyeecheung · 2017-11-15T11:36:05Z

doc/api/errors.md


 <a id="ERR_SOCKET_BAD_TYPE"></a>
 ### ERR_SOCKET_BAD_TYPE

-Used when an API function expecting a socket type (`udp4` or `udp6`) receives an
-invalid value.
+An API function expecting a socket type (`udp4` or `udp6`) receivee an invalid


There is an extra e in receivee

joyeecheung · 2017-11-15T11:36:28Z

doc/api/errors.md

-Used when an attempt is made to close the `process.stderr` stream. By design,
-Node.js does not allow `stdout` or `stderr` Streams to be closed by user code.
+An attempt was made to close the `process.stderr` stream. By design, Node.js
+does not allow `stdout` or `stderr` Streams to be closed by user code.


s/Streams/streams?

joyeecheung · 2017-11-15T11:36:43Z

doc/api/errors.md

-Used when an attempt is made to close the `process.stdout` stream. By design,
-Node.js does not allow `stdout` or `stderr` Streams to be closed by user code.
+An attempt was made to close the `process.stdout` stream. By design, Node.js
+does not allow `stdout` or `stderr` Streams to be closed by user code.


s/Streams/streams?

joyeecheung · 2017-11-15T11:39:02Z

doc/api/errors.md


 <a id="ERR_UNESCAPED_CHARACTERS"></a>
 ### ERR_UNESCAPED_CHARACTERS

-Used when a string that contains unescaped characters was received.
+A string that contains unescaped characters was received.


s/contains/contained/?

joyeecheung · 2017-11-15T11:40:40Z

doc/api/errors.md


 <a id="ERR_UNKNOWN_FILE_EXTENSION"></a>
 ### ERR_UNKNOWN_FILE_EXTENSION

 > Stability: 1 - Experimental

-Used when attempting to load a module with an unknown or unsupported file
-extension.
+Attempting to load a module with an unknown or unsupported file extension.


The previous descriptions are using an attempt was made to..., we may want to be consistent here.

joyeecheung · 2017-11-15T12:20:16Z

doc/api/errors.md


 <a id="ERR_CRYPTO_HASH_DIGEST_NO_UTF16"></a>
 ### ERR_CRYPTO_HASH_DIGEST_NO_UTF16

-Used when the UTF-16 encoding is used with [`hash.digest()`][]. While the
+The UTF-16 encoding was used with [`hash.digest()`][]. While the 


Extra space at the end.

joyeecheung · 2017-11-15T12:20:45Z

doc/api/errors.md


 <a id="ERR_CRYPTO_FIPS_UNAVAILABLE"></a>
 ### ERR_CRYPTO_FIPS_UNAVAILABLE

-Used when trying to enable or disable FIPS mode when FIPS is not available.
+There was an attempt to enable or disable FIPS mode, but FIPS mode is not


For consistency, An attempt was made to enable...but FIPS mode was not available.

joyeecheung · 2017-11-15T12:21:23Z

doc/api/errors.md


 <a id="ERR_ASYNC_TYPE"></a>
 ### ERR_ASYNC_TYPE

-Used when the type of an asynchronous resource is invalid. Note that users are
-also able to define their own types when using the public embedder API.
+The type of an asynchronous resource is invalid. Note that users are also able


s/is/was/

joyeecheung · 2017-11-15T12:21:39Z

doc/api/errors.md

-Used when the type of an asynchronous resource is invalid. Note that users are
-also able to define their own types when using the public embedder API.
+The type of an asynchronous resource is invalid. Note that users are also able
+to define their own types when using the public embedder API.


Maybe drop the when?

I think I'll go with if instead.

joyeecheung · 2017-11-15T12:22:23Z

doc/api/errors.md

@@ -599,182 +599,180 @@ found [here][online].
 <a id="ERR_ARG_NOT_ITERABLE"></a>
 ### ERR_ARG_NOT_ITERABLE

-Used generically to identify that an iterable argument (i.e. a value that works
-with `for...of` loops) is required, but not provided to a Node.js API.
+An iterable argument (i.e. a value that works with `for...of` loops) is


s/is/was/

Remove the practice of starting most error descriptions with "Used when" or wordier variations. Change errors of the form: > Used when the type of an asynchronous resource is invalid. ...to: > The type of an asynchronous resource was invalid. Change errors of the form: > The `'ERR_INVALID_CURSOR_POS'` is thrown specifically when a cursor on > a given stream is attempted to move to a specified row without a specified > column. ...to: > A cursor on a given stream cannot be moved to a specified row without > a specified column.

Trott · 2017-11-15T23:28:14Z

Addressed almost all of the awesome nits from @joyeecheung. PTAL

joyeecheung

Thanks for bearing with me :D

Trott · 2017-11-17T00:28:23Z

CI: https://ci.nodejs.org/job/node-test-pull-request/11488/

Remove the practice of starting most error descriptions with "Used when" or wordier variations. Change errors of the form: > Used when the type of an asynchronous resource is invalid. ...to: > The type of an asynchronous resource was invalid. Change errors of the form: > The `'ERR_INVALID_CURSOR_POS'` is thrown specifically when a cursor on > a given stream is attempted to move to a specified row without a > specified column. ...to: > A cursor on a given stream cannot be moved to a specified row without > a specified column. PR-URL: nodejs#16954 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Trott · 2017-11-17T17:58:14Z

Landed in 959c425.

MylesBorins · 2017-12-11T22:10:28Z

Should this be backported to v9.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

Remove the practice of starting most error descriptions with "Used when" or wordier variations. Change errors of the form: > Used when the type of an asynchronous resource is invalid. ...to: > The type of an asynchronous resource was invalid. Change errors of the form: > The `'ERR_INVALID_CURSOR_POS'` is thrown specifically when a cursor on > a given stream is attempted to move to a specified row without a > specified column. ...to: > A cursor on a given stream cannot be moved to a specified row without > a specified column. PR-URL: nodejs#16954 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Trott · 2017-12-11T23:33:35Z

@MylesBorins Backport is in #17622

Remove the practice of starting most error descriptions with "Used when" or wordier variations. Change errors of the form: > Used when the type of an asynchronous resource is invalid. ...to: > The type of an asynchronous resource was invalid. Change errors of the form: > The `'ERR_INVALID_CURSOR_POS'` is thrown specifically when a cursor on > a given stream is attempted to move to a specified row without a > specified column. ...to: > A cursor on a given stream cannot be moved to a specified row without > a specified column. Backport-PR-URL: #17622 PR-URL: #16954 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

gibfahn · 2017-12-19T22:44:53Z

@Trott backports for v6.x and v8.x? Feel free to mark don't land if it's a hassle.

Trott · 2017-12-19T23:04:19Z

@Trott backports for v6.x and v8.x? Feel free to mark don't land if it's a hassle.

Node.js error codes don't exist in 6.x so marking as dont-land for that version.

They exist in 8.x so I'll take a closer look at that...

Remove the practice of starting most error descriptions with "Used when" or wordier variations. Change errors of the form: > Used when the type of an asynchronous resource is invalid. ...to: > The type of an asynchronous resource was invalid. Change errors of the form: > The `'ERR_INVALID_CURSOR_POS'` is thrown specifically when a cursor on > a given stream is attempted to move to a specified row without a > specified column. ...to: > A cursor on a given stream cannot be moved to a specified row without > a specified column. PR-URL: nodejs#16954 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Trott · 2017-12-19T23:14:38Z

@gibfahn 8.x backport in #17767 and assigned to you.

Remove the practice of starting most error descriptions with "Used when" or wordier variations. Change errors of the form: > Used when the type of an asynchronous resource is invalid. ...to: > The type of an asynchronous resource was invalid. Change errors of the form: > The `'ERR_INVALID_CURSOR_POS'` is thrown specifically when a cursor on > a given stream is attempted to move to a specified row without a > specified column. ...to: > A cursor on a given stream cannot be moved to a specified row without > a specified column. PR-URL: #16954 Backport-PR-URL: #17767 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Trott added doc Issues and PRs related to the documentations. errors Issues and PRs related to JavaScript errors originated in Node.js core. labels Nov 11, 2017

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 11, 2017

maclover7 reviewed Nov 11, 2017

View reviewed changes

jasnell approved these changes Nov 12, 2017

View reviewed changes

gireeshpunathil approved these changes Nov 12, 2017

View reviewed changes

benjamingr requested changes Nov 12, 2017

View reviewed changes

Trott force-pushed the used-when branch from 6e1bab8 to 37b6913 Compare November 12, 2017 14:00

benjamingr approved these changes Nov 12, 2017

View reviewed changes

Trott mentioned this pull request Nov 14, 2017

crypto: Better docs for cases where peer's public key is invalid #16849

Closed

4 tasks

tniessen approved these changes Nov 15, 2017

View reviewed changes

joyeecheung reviewed Nov 15, 2017

View reviewed changes

Trott added 6 commits November 15, 2017 14:38

squash: fix errors that have landed since this opened

b9e372c

squash: cpuUsage nit from maclover7

2edcd00

squash: comma nits from maclover7

5fadfe0

squash: HTTP/2 nit from maclover7

b9fd330

squash: mega-nits from joyee

f85b90d

Trott force-pushed the used-when branch from cb2ece2 to f85b90d Compare November 15, 2017 23:28

joyeecheung approved these changes Nov 16, 2017

View reviewed changes

Trott closed this Nov 17, 2017

MylesBorins added the backport-requested-v9.x label Dec 11, 2017

Trott mentioned this pull request Dec 11, 2017

[v9.x backport] doc: make error descriptions more concise #17622

Closed

4 tasks

Trott removed the backport-requested-v9.x label Dec 11, 2017

MylesBorins added the backported-to-v9.x label Dec 12, 2017

MylesBorins mentioned this pull request Dec 12, 2017

v9.3.0 proposal #17631

Merged

gibfahn added backport-requested-v6.x labels Dec 19, 2017

Trott added dont-land-on-v6.x and removed backport-requested-v6.x labels Dec 19, 2017

Trott mentioned this pull request Dec 19, 2017

[v8.x backport] doc: make error descriptions more concise #17767

Closed

3 tasks

gibfahn added backported-to-v8.x and removed backport-requested-v8.x labels Dec 20, 2017

gibfahn mentioned this pull request Dec 20, 2017

v8.9.4 proposal #17772

Closed

gibfahn mentioned this pull request Dec 20, 2017

v8.9.4 proposal #17774

Merged

Trott deleted the used-when branch January 13, 2022 22:47

doc: make error descriptions more concise #16954

doc: make error descriptions more concise #16954

Conversation

Trott commented Nov 11, 2017

Checklist

Affected core subsystem(s)

maclover7 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasnell left a comment

Choose a reason for hiding this comment

benjamingr left a comment

Choose a reason for hiding this comment

Trott commented Nov 12, 2017

maclover7 commented Nov 12, 2017

benjamingr commented Nov 12, 2017

Trott commented Nov 14, 2017

joyeecheung left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Trott commented Nov 15, 2017

joyeecheung left a comment

Choose a reason for hiding this comment

Trott commented Nov 17, 2017

Trott commented Nov 17, 2017

MylesBorins commented Dec 11, 2017

Trott commented Dec 11, 2017

gibfahn commented Dec 19, 2017

Trott commented Dec 19, 2017

Trott commented Dec 19, 2017