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

Unhandled rejection from `.map()` and `.filter()` in v3.5.1 #1487

Closed
overlookmotel opened this issue Dec 9, 2017 · 5 comments

Comments

@overlookmotel
Copy link
Contributor

commented Dec 9, 2017

Please answer the questions the best you can:

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

3.5.1

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

Node 6.12.2, Mac OS 10.12.6

  1. Did this issue happen with earlier version of bluebird?

No. Issue not present in bluebird 3.5.0.


Chaining .map() or .filter() on to a rejected promise leads to an unhandled rejection, even when .catch() handler is attached further down the chain.

Promise.reject( new Error('foo') )
    .map( () => {} )
    .catch( () => {} );

With bluebird v3.5.1, the above produces Unhandled rejection Error: foo. In v3.5.0 no unhandled rejection is created.

I assume is caused by 48c8591 but I can't understand why!

Don't know if this is related to #1468.

@overlookmotel

This comment has been minimized.

Copy link
Contributor Author

commented Dec 9, 2017

Promise.map() and Promise.filter() show the same problem.

This produces an unhandled rejection:

const p = Promise.reject( new Error('foo') );
Promise.map(p, () => {} )
    .catch( () => {} );
@overlookmotel

This comment has been minimized.

Copy link
Contributor Author

commented Dec 14, 2017

Having looked into this further, I think this problem was actually introduced a while back in a PR that I submitted: #1221

That PR introduced a call to async.invoke() immediately on calling Promise.map(). So no handlers are added to the input promise in that tick. Hence if the input promise is rejected that it is indeed an unhandled rejection.

I guess what should be happening is that the input promise should be checked for if it's already rejected and if it is the promise returned by Promise.map() immediately rejected. And only after that, iterating over the array should be delayed by async.invoke().

@overlookmotel

This comment has been minimized.

Copy link
Contributor Author

commented Dec 14, 2017

Actually, the tick should be introduced after the first round of iteration, but before the map handers are called. Otherwise there can also be unhandled exceptions on any rejected promises in the input array.

At present, the following also creates an unhandled exception:

const arr = [ Promise.reject( new Error('foo') ) ];
Promise.map( arr, () => {} ).catch( () => {} );
@overlookmotel

This comment has been minimized.

Copy link
Contributor Author

commented Dec 20, 2017

Please see PR #1489 which fixes this.

@aviranco

This comment has been minimized.

Copy link

commented Jun 17, 2018

I've experienced this issue as well. What is the status of this issue? the fix is planned to be merged soon?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.