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

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.

@benjamingr
Copy link
Collaborator

Can you add tests?

What about two rejections in a single array?

@overlookmotel
Copy link
Contributor Author

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
Copy link
Contributor Author

@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
Copy link
Contributor Author

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

petkaantonov added a commit that referenced this pull request May 25, 2019
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

Successfully merging this pull request may close these issues.

Unhandled rejection from .map() and .filter() in v3.5.1
3 participants