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

Fix Promise.map unhandled rejections (#1487) #1489

Conversation

@overlookmotel
Copy link
Contributor

commented Dec 20, 2017

This PR fixes #1487.

Previously, unhandled rejection errors can occur with Promise.map(), Promise.filter(), .map() and .filter() when in fact the rejections are handled.

Prior to this PR, all of the following create an unhandled rejection, after this PR they do not.

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

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

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

var arr = [ Promise.reject( new Error('foo') ) ];
var p = Promise.resolve( arr );
p.map( () => {} ).catch( () => {} );


var p = Promise.reject( new Error('foo') );
Promise.filter( p, () => {} ).catch( () => {} );

var arr = [ Promise.reject( new Error('foo') ) ];
Promise.filter( arr, () => {} ).catch( () => {} );

var p = Promise.reject( new Error('foo') );
p.filter( () => {} ).catch( () => {} );

var arr = [ Promise.reject( new Error('foo') ) ];
var p = Promise.resolve( arr );
p.filter( () => {} ).catch( () => {} );

I have not written tests as I'm not sure how to test for unhandled rejections.

@overlookmotel overlookmotel force-pushed the overlookmotel:fix-map-unhandled-rejection branch from 239664f to 3bb03d0 Dec 21, 2017
@benjamingr

This comment has been minimized.

Copy link
Collaborator

commented Dec 21, 2017

Can you add tests?

What about two rejections in a single array?

@overlookmotel

This comment has been minimized.

Copy link
Contributor Author

commented Dec 21, 2017

Would be happy to write tests, but can you advise how to write a test for an unhandled rejection? By it's nature it's hard to test for.

2 rejections in a single array should also be handled as iteration over the array now happens synchronously.

@overlookmotel

This comment has been minimized.

Copy link
Contributor Author

commented Dec 28, 2017

@benjamingr Tests added. I've used ._isRejectionUnhandled() to check that the rejections are handled synchronously. Seems a bit wonky to be using private undocumented APIs in tests, but I can't see any other reasonable way to do it.

@overlookmotel

This comment has been minimized.

Copy link
Contributor Author

commented Dec 28, 2017

NB The test also covers case of 2 rejections in an array.

petkaantonov added a commit that referenced this pull request May 25, 2019
This was referenced Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.