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

Promise.map should generate an error if 'options' is provided but not an object #1097

Closed
Unibozu opened this Issue May 12, 2016 · 4 comments

Comments

Projects
None yet
4 participants
@Unibozu

Unibozu commented May 12, 2016

  1. What version of bluebird is the issue happening on? 3.3.5
  2. What platform and version? NodeJS 5.7.1

Promise.map() takes as a third parameter an options object accepting a single property to define the iteration concurrency limit. Because there is only one option I often mistake this object as a concurrency argument and provide an integer instead of the right object.

Bluebird silently ignores the bad parameter and keeps an infinite concurrency witch causes unknown stress to the server if iterating over a huge dataset. It should throw an exception if the parameter is truthy but not an object to avoid such mistakes.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin May 12, 2016

It's always tricky to find and prevent invalid calls to an API at runtime, there are so many ways to mess things up. However, I think the DefinitelyTyped definitions for Bluebird would have caught this error at compile time if you were using TypeScript. It would even warn you if the object had invalid options.

dmethvin commented May 12, 2016

It's always tricky to find and prevent invalid calls to an API at runtime, there are so many ways to mess things up. However, I think the DefinitelyTyped definitions for Bluebird would have caught this error at compile time if you were using TypeScript. It would even warn you if the object had invalid options.

@benjamingr

This comment has been minimized.

Show comment
Hide comment
@benjamingr

benjamingr May 12, 2016

Collaborator

+1 let's do this

Collaborator

benjamingr commented May 12, 2016

+1 let's do this

@Unibozu

This comment has been minimized.

Show comment
Hide comment
@Unibozu

Unibozu May 12, 2016

@dmethvin indeed. In this case a simple check can be envisaged on the argument itself at least

Unibozu commented May 12, 2016

@dmethvin indeed. In this case a simple check can be envisaged on the argument itself at least

@Unibozu

This comment has been minimized.

Show comment
Hide comment
@Unibozu

Unibozu May 17, 2016

Awesome! :)

Unibozu commented May 17, 2016

Awesome! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment