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

Unable to chain promisifyAll request function #470

Closed
jhnlsn opened this issue Jan 27, 2015 · 4 comments
Closed

Unable to chain promisifyAll request function #470

jhnlsn opened this issue Jan 27, 2015 · 4 comments

Comments

@jhnlsn
Copy link

jhnlsn commented Jan 27, 2015

var Promise   = require('bluebird'),
    request   = Promise.promisifyAll(require('request'))
Promise.props({"url":"http:google.com"}).then(request.getAsync).spread(function(response, body){
});

the above throws a TypeError

Possibly unhandled TypeError: Cannot call method 'get' of undefined
    at ret (eval at makeNodePromisifiedEval (....../node_modules/bluebird/js/main/promisify.js:177:12), <anonymous>:15:22)

however, if I modify it then it works

Promise.props({"url":"http:google.com"}).then(function(result){
return request.getAsync(result);
}).spread(function(response, body){
console.log(body);
});
@petkaantonov
Copy link
Owner

Because promisifyAll is also used to promisify modules and classes whose instances don't exist at promisification time, the methods cannot be bound to any particular this value even though in the singleton case (request, fs etc) that is possible. So they use whatever this is at call time, which is request when doing request.getAsync() but is undefined when doing var get = request.getAsync; get() as per how javascript this works.

So it would need to be an option to promisifyAll (e.g. promisifyAll(..., {singleton: true}))

@benjamingr
Copy link
Collaborator

@petkaantonov calling it a singleton is rather misleading (it's just a namespace and not really a singleton) - what about {bindContext: true}?

@jhnlsn jhnlsn closed this as completed Jan 28, 2015
@jhnlsn jhnlsn reopened this Jan 28, 2015
@jhnlsn
Copy link
Author

jhnlsn commented Jan 28, 2015

Sorry fat thumbed thus on my phone.

@petkaantonov
Copy link
Owner

@johnymonster so what is your opinion? Would you use a bindContext option to be able to pass request.getAsync directly into a then?

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

No branches or pull requests

3 participants