assert: provide extra context in case of failed error validation checks#28263
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
This improves `assert.throws()` and `assert.rejects()` in case error classes are used as validation for the expected error. In case of a failure it will now throw an `AssertionError` instead of the received error if the check fails. That error has the received error as actual property and the expected property is set to the expected error class. The error message should help users to identify the problem faster than before by providing extra context what went wrong.
This makes sure the `generatedMessage` property is always set as expected. This was not the case some `assert.throws` and `assert.rejects` calls.
This makes sure that validation function used by `assert.throws` and `assert.rejects` always throw validatin errors instead of rethrowing the received error. That should improve the debugging experience for developers since they have a better context where the error is coming from and they also get to know what triggered it.
This refactors some code for less duplication.
This adds information about the actual thrown error to the AssertionError's message property. It also improves the logged error instances error name by using the constructors name, if available.
This makes sure that using `assert.throws()` or `assert.rejects()` in combination with Error classes log appropriate error messages in case the expected and received constructor name are identical but not part of the same prototype chain.
BridgeAR added the semver-minor 
nodejs-github-bot added the assert This updates two outdated examples to the current implementation.
This makes sure the current validation function example reflects the current implementation.
Trott reviewed Jun 17, 2019
Uh oh!
There was an error while loading. Please reload this page.
Trott reviewed Jun 17, 2019
Uh oh!
There was an error while loading. Please reload this page.
Trott reviewed Jun 17, 2019
Uh oh!
There was an error while loading. Please reload this page.
This makes sure that `AssertionError` links to the correct place in the assert documentation.
MemberAuthor
BridgeAR commented Jun 20, 2019
@Trott I just updated the PR and incorporated your feedback :) |
Collaborator
nodejs-github-bot commented Jun 20, 2019
MemberAuthor
BridgeAR commented Jun 22, 2019
@nodejs/assert PTAL |
Trott approved these changes Jun 22, 2019
Member
Trott left a comment
Trott left a comment There was a problem hiding this comment.
LGTM but worth a CITGM run out of extra caution, I think.
CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1890/
MemberAuthor
BridgeAR commented Jun 26, 2019
The failures in CITGM are related to esm. I can't see anything related to this PR. |
BridgeAR added author ready 
Member
Trott commented Jul 6, 2019
@nodejs/collaborators Theoretically, this can land, but it would be nice to have a few other eyes on it. Anyone up for reviewing it? |
benjamingr approved these changes Jul 6, 2019
Member
boneskull commented Jul 6, 2019
I’m a bit confused. before this change, |
Trott removed the author ready Member
Trott commented Jul 7, 2019
Yes, that's correct. |
Trott added the author ready Member
boneskull commented Jul 7, 2019
If that’s the case, this seems like a breaking change. |
BethGriggs added a commit that referenced this pull request Oct 22, 2019
Notable changes: * assert: * If the validation function passed to `assert.throws()` or `assert.rejects()` returns a value other than `true`, an assertion error will be thrown instead of the original error to highlight the programming mistake (Ruben Bridgewater). #28263 * If a constructor function is passed to validate the instance of errors thrown in `assert.throws()` or `assert.reject()`, an assertion error will be thrown instead of the original error (Ruben Bridgewater). #28263 * build: * Node.js releases are now built with default full-icu support. This means that all locales supported by ICU are now included and Intl-related APIs may return different values than before (Richard Lau). #29887 * The minimum Xcode version supported for macOS was increased to 10. It is still possible to build Node.js with Xcode 8 but this may no longer be the case in a future v13.x release (Michael Dawson). #29622 * child_process: * `ChildProcess._channel` (DEP0129) is now a Runtime deprecation (cjihrig). #27949 * console: * The output `console.timeEnd()` and `console.timeLog()` will now automatically select a suitable time unit instead of always using milliseconds (Xavier Stouder). #29251 * deps: * The V8 engine was updated to version 7.8. This includes performance improvements to object destructuring, memory usage and WebAssembly startup time (Myles Borins). #29694) * domain: * The domain's error handler is now executed with the active domain set to the domain's parent to prevent inner recursion (Julien Gilli). #26211 * fs: * The undocumented method `FSWatcher.prototype.start()` was removed (Lucas Holmquist). #29905 * Calling the `open()` method on a `ReadStream` or `WriteStream` now emits a runtime deprecation warning. The methods are supposed to be internal and should not be called by user code (Robert Nagy). #29061 * `fs.read/write`, `fs.readSync/writeSync` and `fd.read/write` now accept any safe integer as their `offset` parameter. The value of `offset` is also no longer coerced, so a valid type must be passed to the functions (Zach Bjornson). #26572 * http: * Aborted requests no longer emit the `end` or `error` events after `aborted` (Robert Nagy). #27984#20077 * Data will no longer be emitted after a socket error (Robert Nagy). #28711 * The legacy HTTP parser (previously available under the `--http-parser=legacy` flag) was removed (Anna Henningsen). #29589 * The `host` option for HTTP requests is now validated to be a string value (Giorgos Ntemiris). #29568 * The `request.connection` and `response.connection` properties are now runtime deprecated. The equivalent `request.socket` and `response.socket` should be used instead (Robert Nagy). #29015 * http, http2: * The default server timeout was removed (Ali Ijaz Sheikh). #27558 * Brought 425 status code name into accordance with RFC 8470. The name changed from "Unordered Collection" to "Too Early" (Sergei Osipov). #29880 * lib: * The `error.errno` property will now always be a number. To get the string value, use `error.code` instead (Joyee Cheung). #28140 * module: * `module.createRequireFromPath()` is deprecated. Use `module.createRequire()` instead (cjihrig). #27951 * src: * Changing the value of `process.env.TZ` will now clear the tz cache. This affects the default time zone used by methods such as `Date.prototype.toString` (Ben Noordhuis). #20026 * stream: * The timing and behavior of streams was consolidated for a number of edge cases. Please look at the individual commits below for more information. PR-URL: #29504
BethGriggs added a commit that referenced this pull request Oct 22, 2019
Notable changes: * assert: * If the validation function passed to `assert.throws()` or `assert.rejects()` returns a value other than `true`, an assertion error will be thrown instead of the original error to highlight the programming mistake (Ruben Bridgewater). #28263 * If a constructor function is passed to validate the instance of errors thrown in `assert.throws()` or `assert.reject()`, an assertion error will be thrown instead of the original error (Ruben Bridgewater). #28263 * build: * Node.js releases are now built with default full-icu support. This means that all locales supported by ICU are now included and Intl-related APIs may return different values than before (Richard Lau). #29887 * The minimum Xcode version supported for macOS was increased to 10. It is still possible to build Node.js with Xcode 8 but this may no longer be the case in a future v13.x release (Michael Dawson). #29622 * child_process: * `ChildProcess._channel` (DEP0129) is now a Runtime deprecation (cjihrig). #27949 * console: * The output `console.timeEnd()` and `console.timeLog()` will now automatically select a suitable time unit instead of always using milliseconds (Xavier Stouder). #29251 * deps: * The V8 engine was updated to version 7.8. This includes performance improvements to object destructuring, memory usage and WebAssembly startup time (Myles Borins). #29694) * domain: * The domain's error handler is now executed with the active domain set to the domain's parent to prevent inner recursion (Julien Gilli). #26211 * fs: * The undocumented method `FSWatcher.prototype.start()` was removed (Lucas Holmquist). #29905 * Calling the `open()` method on a `ReadStream` or `WriteStream` now emits a runtime deprecation warning. The methods are supposed to be internal and should not be called by user code (Robert Nagy). #29061 * `fs.read/write`, `fs.readSync/writeSync` and `fd.read/write` now accept any safe integer as their `offset` parameter. The value of `offset` is also no longer coerced, so a valid type must be passed to the functions (Zach Bjornson). #26572 * http: * Aborted requests no longer emit the `end` or `error` events after `aborted` (Robert Nagy). #27984#20077 * Data will no longer be emitted after a socket error (Robert Nagy). #28711 * The legacy HTTP parser (previously available under the `--http-parser=legacy` flag) was removed (Anna Henningsen). #29589 * The `host` option for HTTP requests is now validated to be a string value (Giorgos Ntemiris). #29568 * The `request.connection` and `response.connection` properties are now runtime deprecated. The equivalent `request.socket` and `response.socket` should be used instead (Robert Nagy). #29015 * http, http2: * The default server timeout was removed (Ali Ijaz Sheikh). #27558 * Brought 425 status code name into accordance with RFC 8470. The name changed from "Unordered Collection" to "Too Early" (Sergei Osipov). #29880 * lib: * The `error.errno` property will now always be a number. To get the string value, use `error.code` instead (Joyee Cheung). #28140 * module: * `module.createRequireFromPath()` is deprecated. Use `module.createRequire()` instead (cjihrig). #27951 * src: * Changing the value of `process.env.TZ` will now clear the tz cache. This affects the default time zone used by methods such as `Date.prototype.toString` (Ben Noordhuis). #20026 * stream: * The timing and behavior of streams was consolidated for a number of edge cases. Please look at the individual commits below for more information. PR-URL: #29504
4 tasks
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 22, 2020
This makes sure the `generatedMessage` property is always set as expected. This was not the case some `assert.throws` and `assert.rejects` calls. PR-URL: nodejs#28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 22, 2020
This refactors some code for less duplication. PR-URL: nodejs#28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 22, 2020
This updates two outdated examples to the current implementation. PR-URL: nodejs#28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 22, 2020
This makes sure that `AssertionError` links to the correct place in the assert documentation. PR-URL: nodejs#28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
This makes sure the `generatedMessage` property is always set as expected. This was not the case some `assert.throws` and `assert.rejects` calls. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
This refactors some code for less duplication. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
This updates two outdated examples to the current implementation. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
This makes sure that `AssertionError` links to the correct place in the assert documentation. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
This makes sure the `generatedMessage` property is always set as expected. This was not the case some `assert.throws` and `assert.rejects` calls. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
This refactors some code for less duplication. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
This updates two outdated examples to the current implementation. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
This makes sure that `AssertionError` links to the correct place in the assert documentation. Backport-PR-URL: #31431 PR-URL: #28263 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
Merged
twada added a commit to twada/rejected-or-not that referenced this pull request Apr 18, 2020
…rejects() returns a value other than true, an assertion error will be thrown instead of the original error to highlight the programming mistake refs: nodejs/node#28263
twada added a commit to twada/rejected-or-not that referenced this pull request Apr 18, 2020
… errors thrown in assert.throws() or assert.reject(), an assertion error will be thrown instead of the original error refs: nodejs/node#28263
twada added a commit to twada/rejected-or-not that referenced this pull request Apr 18, 2020
…rejects() returns a value other than true, an assertion error will be thrown instead of the original error to highlight the programming mistake refs: nodejs/node#28263
twada added a commit to twada/rejected-or-not that referenced this pull request Apr 18, 2020
… errors thrown in assert.throws() or assert.reject(), an assertion error will be thrown instead of the original error refs: nodejs/node#28263
twada added a commit to twada/rejected-or-not that referenced this pull request Apr 18, 2020
…rejects() returns a value other than true, an assertion error will be thrown instead of the original error to highlight the programming mistake refs: nodejs/node#28263
twada added a commit to twada/rejected-or-not that referenced this pull request Apr 18, 2020
… errors thrown in assert.throws() or assert.reject(), an assertion error will be thrown instead of the original error refs: nodejs/node#28263
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Please have a look at the individual commit messages for a detailed description.
I mark this as semver-minor since it adds a wrapper to the actual triggered error and as such it's something new.
// cc @nodejs/assert
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes