Skip to content

Add assertion error docs #19724

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

Closed
wants to merge 4 commits into from

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Apr 1, 2018

This was never properly documented but it is used in the wild and it makes sense to actually explain how it works and what properties the errors contain. Those properties can be useful for the users.

Please note: I explicitly did not document the errorDiff option as I plan on removing it again in 10.x. It was introduced in v.9.9.x and I think we can still go ahead to remove it as it is unlikely that it is already used.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

BridgeAR added 2 commits April 1, 2018 10:28
The AssertionError was always exposed but never properly documented.
This explains how it is used and what options it accepts.
@BridgeAR BridgeAR requested review from jasnell and Trott April 1, 2018 08:33
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 1, 2018
@BridgeAR
Copy link
Member Author

BridgeAR commented Apr 1, 2018

@BridgeAR BridgeAR requested a review from vsemozhetbyt April 1, 2018 08:33
@vsemozhetbyt vsemozhetbyt added assert Issues and PRs related to the assert subsystem. errors Issues and PRs related to JavaScript errors originated in Node.js core. labels Apr 1, 2018
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt left a comment

Choose a reason for hiding this comment

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

With some nits)

// AssertionError [ERR_ASSERTION]: 1 === 2
```
A subclass of `Error` that indicates the failure of an assertion. For further
details check asserts [`Class: AssertionError`][].
Copy link
Contributor

Choose a reason for hiding this comment

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

asserts?

Copy link
Member

Choose a reason for hiding this comment

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

Add a comma after details and change check to see.

@@ -1680,6 +1675,7 @@ Creation of a [`zlib`][] object failed due to incorrect configuration.
[`process.setUncaughtExceptionCaptureCallback()`]: process.html#process_process_setuncaughtexceptioncapturecallback_fn
[`require('crypto').setEngine()`]: crypto.html#crypto_crypto_setengine_engine_flags
[`server.listen()`]: net.html#net_server_listen
[`Class: AssertionError`]: assert.html#class_assertionerror
Copy link
Contributor

Choose a reason for hiding this comment

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

##assert_class_assertionerror?

Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Apr 1, 2018

Choose a reason for hiding this comment

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

Or, if the suggestions given below are valid, even #assert_class_assert_assertionerror.

## Class: AssertionError

A subclass of `Error` that indicates the failure of an assertion.
All errors thrown by the assert module will be instance of the `AssertionError`
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure, but maybe instance -> instances?

Copy link
Member

Choose a reason for hiding this comment

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

assert module -> `assert` module (add backticks)

@@ -13,6 +13,71 @@ A `strict` and a `legacy` mode exist, while it is recommended to only use
For more information about the used equality comparisons see
[MDN's guide on equality comparisons and sameness][mdn-equality-guide].

## Class: AssertionError
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is not global, should not this be ## Class: assert.AssertionError?

Compare:
Class: net.Server and Class: net.Socket

All errors thrown by the assert module will be instance of the `AssertionError`
class.

### new AssertionError(options)
Copy link
Contributor

Choose a reason for hiding this comment

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

The same.
Compare: new net.Server() and new net.Socket().

[`assert.strictEqual()`] is used.
* `expected` {any} Set to the expected value in case e.g.,
[`assert.strictEqual()`] is used.
* `generatedMessage` {boolean} Indicates if the message was auto generated
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure, but maybe auto generated -> auto-generated?

* `generatedMessage` {boolean} Indicates if the message was auto generated
(`true`) or not.
* `code` {string} This is always set to the string `ERR_ASSERTION` to indicate
that the error is actually a assertion error.
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Apr 1, 2018

Choose a reason for hiding this comment

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

a assertion -> an assertion?

```js
const assert = require('assert');

// Generate a AssertionError to compare the error message later:.
Copy link
Contributor

Choose a reason for hiding this comment

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

a AssertionError -> an AssertionError?

:. -> :?

assert.equal(err.expected, 2);
assert.equal(err.code, 'ERR_ASSERTION');
assert.equal(err.operator, '==');
assert.equal(err.generatedMessage, true);
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt Apr 1, 2018

Choose a reason for hiding this comment

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

Not sure if we should popularize legacy loose equality mode here.
Mabe this way?

const assert = require('assert');

// Generate an AssertionError to compare the error message later:
const { message } = new assert.AssertionError({
  actual: 1,
  expected: 2,
  operator: 'strictEqual'
});

// Verify error output:
try {
  assert.strictEqual(1, 2);
} catch (err) {
  assert(err instanceof assert.AssertionError);
  assert.strictEqual(err.message, message);
  assert.strictEqual(err.name, 'AssertionError [ERR_ASSERTION]');
  assert.strictEqual(err.actual, 1);
  assert.strictEqual(err.expected, 2);
  assert.strictEqual(err.code, 'ERR_ASSERTION');
  assert.strictEqual(err.operator, 'strictEqual');
  assert.strictEqual(err.generatedMessage, true);
}

Or maybe with 1 and '1'?


A subclass of `Error` that indicates the failure of an assertion.

All instances contain the built-in Error properties (i.e., `message` and `name`)
Copy link
Member

Choose a reason for hiding this comment

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

  • Add backticks around Error.
  • Is i.e. the right abbreviation here? If so, then just get rid of i.e.. If not, change it to including or such as.

A subclass of `Error` that indicates the failure of an assertion.

All instances contain the built-in Error properties (i.e., `message` and `name`)
plus the following:
Copy link
Member

Choose a reason for hiding this comment

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

plus the following: -> and:

@Trott
Copy link
Member

Trott commented Apr 1, 2018

Since there's substantial new text here: @nodejs/documentation

@BridgeAR
Copy link
Member Author

BridgeAR commented Apr 1, 2018

Comments addressed.

Lite CI: https://ci.nodejs.org/job/node-test-pull-request-lite/378/

@@ -365,7 +365,7 @@ detailed [here](#errors_system_errors).
## Class: AssertionError

A subclass of `Error` that indicates the failure of an assertion. For further
details check asserts [`Class: AssertionError`][].
details, see [`Class: AssertionError`][].
Copy link
Contributor

Choose a reason for hiding this comment

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

AssertionError -> assert.AssertionError?

@@ -1675,7 +1675,7 @@ Creation of a [`zlib`][] object failed due to incorrect configuration.
[`process.setUncaughtExceptionCaptureCallback()`]: process.html#process_process_setuncaughtexceptioncapturecallback_fn
[`require('crypto').setEngine()`]: crypto.html#crypto_crypto_setengine_engine_flags
[`server.listen()`]: net.html#net_server_listen
[`Class: AssertionError`]: assert.html#class_assertionerror
[`Class: AssertionError`]: assert.html#assert_class_assert_assertionerror
Copy link
Contributor

Choose a reason for hiding this comment

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

AssertionError -> assert.AssertionError?

* `generatedMessage` {boolean} Indicates if the message was auto-generated
(`true`) or not.
* `code` {string} This is always set to the string `ERR_ASSERTION` to indicate
that the error is actually a assertion error.
Copy link
Contributor

Choose a reason for hiding this comment

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

Still a assertion -> an assertion?

assert.strictEqual(1, 2);
// AssertionError [ERR_ASSERTION]: 1 === 2
```
A subclass of `Error` that indicates the failure of an assertion. For further
Copy link
Member

Choose a reason for hiding this comment

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

Microscopic nit that can totally be ignored if you want: The word further can be omitted. For that matter, so can all of For further details,. It can just be See [`Class: AssertionError`][].

// AssertionError [ERR_ASSERTION]: 1 === 2
```
A subclass of `Error` that indicates the failure of an assertion. For further
details, see [`Class: AssertionError`][].
Copy link
Member

Choose a reason for hiding this comment

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

Should it now be Class: assert.AssertionError?

@BridgeAR
Copy link
Member Author

BridgeAR commented Apr 2, 2018

New comments addressed.

New Lite-CI: https://ci.nodejs.org/job/node-test-pull-request-lite/390/

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 2, 2018
BridgeAR added a commit to BridgeAR/node that referenced this pull request Apr 4, 2018
The AssertionError was always exposed but never properly documented.
This explains how it is used and what options it accepts.

PR-URL: nodejs#19724
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Apr 4, 2018
PR-URL: nodejs#19724
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@BridgeAR
Copy link
Member Author

BridgeAR commented Apr 4, 2018

Landed in 5bdd6a7 and 3567ea0

@BridgeAR BridgeAR closed this Apr 4, 2018
@targos targos removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 4, 2018
targos pushed a commit that referenced this pull request Apr 6, 2018
The AssertionError was always exposed but never properly documented.
This explains how it is used and what options it accepts.

PR-URL: #19724
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this pull request Apr 6, 2018
PR-URL: #19724
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@BridgeAR BridgeAR deleted the add-assertion-error-docs branch April 1, 2019 23:40
@BridgeAR BridgeAR restored the add-assertion-error-docs branch April 1, 2019 23:40
@BridgeAR BridgeAR deleted the add-assertion-error-docs branch January 20, 2020 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert Issues and PRs related to the assert subsystem. 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.

7 participants