petkaantonov/bluebird

Not respecting `--unhandled-rejections=strict`

misozask opened this issue · 10 comments

misozask commented

  1. What version of bluebird is the issue happening on?
    3.7.2

  2. What platform and version? (For example Node.js 0.12 or Google Chrome 32)
    Node v14.5.0

Running following code:

setTimeout(() => console.log('not executed'), 1000)

const rejected = new Promise((res, rej) => {
  rej(new Error('error'))
})

with node's native promise and cli option --unhandled-rejections=strict will terminate the script on unhandled rejection.

But when using Bluebirds promise, it will not.

const { Promise } = require('bluebird')

setTimeout(() => console.log('will be executed!'), 1000)

const rejected = new Promise((res, rej) => {
  rej(new Error('error'))
})

It seems that --unhandled-rejections=strict is ignored

benjamingr commented

Why would you expect bluebird to respect a node cli flag?

misozask commented

I was just assuming that it will behave the same, no matter which promise implementation is used.

Also from the warning Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code. so I would expect that it is preferred to terminate the process asap.
I was a bit suprised when I switch to Bluebirds promise and my script kept running ...

benjamingr commented

I'll consult with other node people. One option is to fix this on the Node side. Bluebird is currently unaware of cli options and such - we can define an environment variable but that would be weird.

misozask commented

@benjamingr Is there any outcome? Thanks

benjamingr commented

Well, I had an implementation that changes Node's behavior to implement "strict" by setting an 'unhandledRejection' handler and discussed it with Ruben but I am concerned it's a breaking change since there is a lot of wild code that assumes things regarding the number of "unhandledRejection" handlers there are attached so I am not sure it's worth the breaking change in Node to fix it from that side. (I don't think this can be fixed or changed from the bluebird side)

misozask commented

Thanks, I'm closing it.

  • 👍1
tjconcept commented

@benjamingr purely from a "principle of least surprise" I'd consider it a bug in Node.js that anything showing up in an unhandledRejection handler would not also crash when that flag is set. What does that flag really mean then, if only a subset of those events are considered?

Naturally the trade-offs must be considered. Do you have a reference for the Node.js issue - I'd like to subscribe, just in case.

benjamingr commented

@tjconcept I just talked about it with Ruben on facebook and made a branch ^_^ feel free to open an issue on Node.js - I am just one person in the Node.js project if other people feel strongly enough about it might happen (and I am not sure it's not a good idea - just scary and a breaking change).

tjconcept commented

If there's already a branch, rather a PR from that, no? I don't feel confident enough about this that I'll file an issue (I at least feel responsible to test it myself and include reproducing code, etc.). Thanks anyway!

benjamingr commented

9 out of 10 times the hard part is figuring out what to do and not the code change itself. The code change is trivial (just make strict use unhandledRejection) whereas figuring out if it's worth the breaking change is definitely not clear cut.

Honestly I think it makes sense to delay this covnersation a few months in case the survey results indicate that strict will be the default anyway.

  • 👍1