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

node 15, eventually.throw ? #267

Open
drom opened this issue Oct 24, 2020 · 2 comments
Open

node 15, eventually.throw ? #267

drom opened this issue Oct 24, 2020 · 2 comments

Comments

@drom
Copy link

drom commented Oct 24, 2020

This construct works in Node 10,12,14 and breaks in Node 15

expect( func(arg) ).to.eventually.throw(TypeError);

Real test case:
https://github.com/sifive/duh-core/blob/master/test/invalid.js#L26

Node 12 (worked before)
https://github.com/sifive/duh-core/runs/1301279599?check_suite_focus=true#step:4:49

Node 15 (does not work as expected)
https://github.com/sifive/duh-core/runs/1301279658?check_suite_focus=true#step:4:60

@markstos
Copy link

markstos commented Sep 22, 2021

This behavior change is due to a documented breaking change in Node 15:

Throw on Unhandled Rejections
As of Node.js 15, the default mode for unhandledRejection is changed to throw > (from warn). In throw mode, if an unhandledRejection hook is not set, the
unhandledRejection is raised as an uncaught exception. Users that have an
unhandledRejection hook should see no change in behavior, and it’s still
possible to switch modes using the --unhandled-rejections=mode process flag.

So you can see that the code will still work with Node 15 or 16 if you call it with:

node --unhandled-rejections=warn

You can also workaround it by adding this to your tests, realizing that you are modifying the global environment:

process.on('unhandledRejection', (err, p) => {
  console.error('unhandledRejection', err.stack, p)
})

A global solution to add this to your tests is to create a "bootstrap" file that is loaded before all your tests run and add the code there. But this may cause your code to issue warning when under test but then throw in production. 🤷🏼

mocha --require 'test/lib/bootstrap.js'

@markstos
Copy link

markstos commented Sep 22, 2021

Another option is to quit using chai-as-promised to test rejected promises. This simplifies your code, doesn't require a global handler that might have unintended side-effects and is fairly straightforward.

The pattern is to attach a then handler to your promise that asserts failure if called, then attach a catch/reject handler to the promise that does nothing if called, becoming a "successful" promise in the process. Here's an example:

return Promise.reject(new Error()).then(() => assert.fail(), () => {});

That's using a two-argument call to then where the second argument is the do-nothing rejection handler. If you want to test that a successul promise would fail if used with this patten, here's an example where the assertion will fail because the first promise will successed, triggering assert.fail():

 return Promise.resolve().then(() => assert.fail(), () => {});

This is approach I'm going with. Promises are already complex enough so removing the extra complexity added by the global side-effects of chai-as-promised can be considered an improvement to the code base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants