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

TypeError when a filter in catch handler is not a function #1517

Closed
marijaselakovic opened this issue Apr 25, 2018 · 4 comments
Closed

TypeError when a filter in catch handler is not a function #1517

marijaselakovic opened this issue Apr 25, 2018 · 4 comments

Comments

@marijaselakovic
Copy link

@marijaselakovic marijaselakovic commented Apr 25, 2018

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

  2. What platform and version? (For example Node.js 0.12 or Google Chrome 32)
    Node.js v8.5, Ubuntu 16.04.4

  3. Did this issue happen with earlier version of bluebird?
    Not sure

If a non-function value is passed as a filter to catch I get the following error: TypeError: Cannot read property 'apply' of undefined
The following code demonstrates this:

var p2 = Promise.reject(10);
p2.catch(function(){return true},undefined)

You should consider adding a type check on arguments passed to catch.

@benjamingr
Copy link
Collaborator

@benjamingr benjamingr commented Apr 25, 2018

@marijaselakovic honestly I just use TS which catches that - a PR is welcome I believe though.

cc @petkaantonov

@marijaselakovic
Copy link
Author

@marijaselakovic marijaselakovic commented Apr 25, 2018

Sure, PR 1518

@benjamingr
Copy link
Collaborator

@benjamingr benjamingr commented Apr 25, 2018

@marijaselakovic sure, can you add a test to the PR that shows the difference in the error thrown?

@marijaselakovic
Copy link
Author

@marijaselakovic marijaselakovic commented Apr 25, 2018

Yes, I added the test, which would fail in both versions, but you can see the difference in error messages. Not sure if that's enough

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants