GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — assert: provide extra context in case of failed error validation checks by BridgeAR · Pull Request #28263 · nodejs/node

Conversation

@BridgeAR

@BridgeAR BridgeAR commented Jun 17, 2019 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jaydenseric reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
BridgeAR added 6 commits June 15, 2019 14:25
@BridgeAR
assert: improve class instance errors
75e5ba1
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.
@BridgeAR
assert: fix generatedMessage property
f90cfca
This makes sure the `generatedMessage` property is always set as
expected. This was not the case some `assert.throws` and
`assert.rejects` calls.
@BridgeAR
assert: wrap validation function errors
4d29fd1
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.
@BridgeAR
assert: DRY .throws code
1563d9d
This refactors some code for less duplication.
@BridgeAR
assert: add more information to AssertionErrors
9d7d103
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.
@BridgeAR
assert: special handle identical error names in instance checks
adb44d3
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 BridgeAR added the semver-minor PRs that contain new features and should be released in the next minor version. label Jun 17, 2019
@BridgeAR
BridgeAR requested a review from Trott June 17, 2019 09:48
@nodejs-github-bot nodejs-github-bot added the assert Issues and PRs related to the assert subsystem. label Jun 17, 2019
BridgeAR added 2 commits June 17, 2019 16:35
@BridgeAR
doc: update assert.throws() examples
e096890
This updates two outdated examples to the current implementation.
@BridgeAR
doc: update assert.throws example
fa0f3f1
This makes sure the current validation function example reflects the
current implementation.
Trott
Trott reviewed Jun 17, 2019
View reviewed changes
Comment thread doc/api/assert.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

Trott
Trott reviewed Jun 17, 2019
View reviewed changes
Comment thread doc/api/assert.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

Trott
Trott reviewed Jun 17, 2019
View reviewed changes
Comment thread doc/api/assert.md Resolved

Uh oh!

There was an error while loading. Please reload this page.

BridgeAR added 2 commits June 20, 2019 17:13
@BridgeAR
doc: make AssertionError a link
683e311
This makes sure that `AssertionError` links to the correct place in
the assert documentation.
@BridgeAR
fixup: doc: update assert.throws example
9832d3d
@BridgeAR

BridgeAR commented Jun 20, 2019

Copy link
Copy Markdown
Member Author

@Trott I just updated the PR and incorporated your feedback :)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nodejs-github-bot

nodejs-github-bot commented Jun 20, 2019

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@BridgeAR

BridgeAR commented Jun 22, 2019

Copy link
Copy Markdown
Member Author

@nodejs/assert PTAL

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Trott
Trott approved these changes Jun 22, 2019
View reviewed changes

@Trott Trott left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@BridgeAR

BridgeAR commented Jun 26, 2019

Copy link
Copy Markdown
Member Author

The failures in CITGM are related to esm. I can't see anything related to this PR.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@BridgeAR BridgeAR added author ready PRs with CI started, the required approvals, and no outstanding review comments. review wanted PRs that need review. labels Jun 26, 2019
@Trott

Trott commented Jul 6, 2019

Copy link
Copy Markdown
Member

@nodejs/collaborators Theoretically, this can land, but it would be nice to have a few other eyes on it. Anyone up for reviewing it?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

benjamingr
benjamingr approved these changes Jul 6, 2019
View reviewed changes
@boneskull

boneskull commented Jul 6, 2019

Copy link
Copy Markdown
Member

I’m a bit confused. before this change, throws would throw the error which was thrown if the function does not throw with the expected class of error? and now it throws assertionerror?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Trott Trott removed the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Jul 7, 2019
@Trott

Trott commented Jul 7, 2019

Copy link
Copy Markdown
Member

I’m a bit confused. before this change, throws would throw the error which was thrown if the function does not throw with the expected class of error? and now it throws assertionerror?

Yes, that's correct.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Trott Trott added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Jul 7, 2019
@boneskull

boneskull commented Jul 7, 2019

Copy link
Copy Markdown
Member

If that’s the case, this seems like a breaking change.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

BethGriggs added a commit that referenced this pull request Oct 22, 2019
@BethGriggs
2019-10-22, Version 13.0.0 (Current)
111e59b
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
@BethGriggs
2019-10-22, Version 13.0.0 (Current)
42bf0c8
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
@BridgeAR
BridgeAR deleted the improve-assert-throws-rejects-errors branch January 20, 2020 12:05
@BridgeAR BridgeAR mentioned this pull request Jan 22, 2020
[v12.x] multiple util and assert backports #31431
Closed
4 tasks done
BridgeAR added a commit to BridgeAR/node that referenced this pull request Jan 22, 2020
@BridgeAR
assert: fix generatedMessage property
32dfdb8
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
@BridgeAR
assert: DRY .throws code
ad12514
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
@BridgeAR
doc: update assert.throws() examples
8b82c85
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
@BridgeAR
doc: make AssertionError a link
ed2d864
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]>
@BridgeAR BridgeAR added the backport-open-v12.x label Jan 22, 2020
MylesBorins pushed a commit that referenced this pull request Jan 30, 2020
@BridgeAR @MylesBorins
assert: fix generatedMessage property
e938784
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
@BridgeAR @MylesBorins
assert: DRY .throws code
881ebce
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
@BridgeAR @MylesBorins
doc: update assert.throws() examples
6d8a541
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
@BridgeAR @MylesBorins
doc: make AssertionError a link
5c6a6b8
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]>
@MylesBorins MylesBorins added backported-to-v12.x and removed backport-open-v12.x labels Jan 30, 2020
@targos targos mentioned this pull request Jan 30, 2020
v12.15.0 release proposal #31368
Closed
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
@BridgeAR @BethGriggs
assert: fix generatedMessage property
749bc16
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
@BridgeAR @BethGriggs
assert: DRY .throws code
7d6c963
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
@BridgeAR @BethGriggs
doc: update assert.throws() examples
08b5a2f
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
@BridgeAR @BethGriggs
doc: make AssertionError a link
9a6aff8
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]>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
v12.16.0 proposal #31691
Merged
twada added a commit to twada/rejected-or-not that referenced this pull request Apr 18, 2020
@twada
feat: If the validation function passed to assert.throws() or assert.…
41fc415
…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
@twada
feat: If a constructor function is passed to validate the instance of…
2306eec
… errors thrown in assert.throws() or assert.reject(), an assertion error will be thrown instead of the original error

refs: nodejs/node#28263
@twada twada mentioned this pull request Apr 18, 2020
Sync with behavior changes in Node13 twada/rejected-or-not#4
Merged
twada added a commit to twada/rejected-or-not that referenced this pull request Apr 18, 2020
@twada
feat: If the validation function passed to assert.throws() or assert.…
7964da1
…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
@twada
feat: If a constructor function is passed to validate the instance of…
ed2353d
… 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
@twada
feat: If the validation function passed to assert.throws() or assert.…
6624cc1
…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
@twada
feat: If a constructor function is passed to validate the instance of…
aa73271
… 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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes
@Trott Trott Trott approved these changes
@benjamingr benjamingr benjamingr approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

assert Issues and PRs related to the assert subsystem. author ready PRs with CI started, the required approvals, and no outstanding review comments. notable-change PRs with changes that should be highlighted in changelogs. semver-major PRs that contain breaking changes and should be released in the next major version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@BridgeAR @nodejs-github-bot @Trott @boneskull @addaleax @jasnell @benjamingr @MylesBorins
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.