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

assert.throws works incorrectly for functions that throw undefined #18027

Closed
not-an-aardvark opened this issue Jan 7, 2018 · 2 comments
Closed
Labels
assert Issues and PRs related to the assert subsystem. confirmed-bug Issues with confirmed bugs.

Comments

@not-an-aardvark
Copy link
Contributor

  • Version: 9.3.0
  • Platform: macOS
  • Subsystem: assert

assert.throws and assert.doesNotThrow seem to treat functions that throw undefined the same way as functions that do not throw a value.

'use strict';

const assert = require('assert');

function foo() {
  throw undefined;
}

assert.throws(foo);

Expected behavior: assertion succeeds

Actual behavior: assertion fails

@not-an-aardvark not-an-aardvark added assert Issues and PRs related to the assert subsystem. confirmed-bug Issues with confirmed bugs. labels Jan 7, 2018
bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Jan 7, 2018
And make `assert.doesNotThrow()` handle it as well.

Fixes: nodejs#18027
@bnoordhuis
Copy link
Member

#18029

@BridgeAR
Copy link
Member

BridgeAR commented Jan 7, 2018

As far as I see it, it was never really supported to do this. In Node.js < 8.3 all falsy values failed. Afterwards only undefined failed.

BridgeAR pushed a commit to BridgeAR/node that referenced this issue Mar 8, 2018
And make `assert.doesNotThrow()` handle it as well.

PR-URL: nodejs#18029
Fixes: nodejs#18027
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Mar 15, 2018
And make `assert.doesNotThrow()` handle it as well.

Backport-PR-URL: #19230
PR-URL: #18029
Fixes: #18027
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this issue Mar 20, 2018
And make `assert.doesNotThrow()` handle it as well.

Backport-PR-URL: #19230
PR-URL: #18029
Fixes: #18027
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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. confirmed-bug Issues with confirmed bugs.
Projects
None yet
Development

No branches or pull requests

3 participants