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

Promisify function that returns some value and takes callback for result/error #402

Closed
thesebas opened this issue Dec 11, 2014 · 13 comments
Closed

Comments

@thesebas
Copy link

I think that promisifying function that returns some value and takes callback for result/error makes result of this function lost, lets say there is some function that returns object for controlling this async process:

function doAsync(params, cb){
  ///...
  return {
    abort: function(){ /* ... */},
    // ....
  }
}

var process = doAsync({/* some params */}, function(err, result){ 
 // handle result or err 
});

// something happens
process.abort();

now after promisifying this function I can't access this process object

var doAsyncPromised = Promise.promisify(doAsync);
doAsyncPromised({/* some params */})
  .then(function(result){
    // handle result
  }, function(err){
    // handle err
  });

so what I'm missing here is some callback that I could intercept this returned value

doAsyncPromised({/* some params */})
  .intercept(function(process){
    // something happens
    process.abort();
  })
  .then(function(result){
    // ...
  });

Maybe creating functions that returns value and calls callback is not the best pattern but some libraries doing that already exists (for example request).

@phpnode
Copy link

phpnode commented Dec 11, 2014

@thesebas yikes, that is pretty horrible, almost a reinvention of promises. In such cases it's better to write your own promisifier I think, this cannot be a common scenario.

@thesebas
Copy link
Author

Well, as I said there is request library that has those methods request.post , request.get that returns request object you can manipulate and/or inspect after calling

var request = require('request');
var req = request.post( params, callback ); // returns req object I can abort with `req.abort()`

but after promisifying this lib

var request = require('request');
Promise.promisifyAll(request)

request.postAsync(params) // ->returns Promise, can't inspect request in progress, 
                          // can't abort it, etc...

and I think I could find more examples, but request is the one I'm working with now.

@benjamingr
Copy link
Collaborator

As stated in the documentation and previous issues automatic promisifcation only works for method explicitly abiding the nodeback contract (including no return value).

Indeed the correct way to address this is to write your own promisifier for promisifyAll (it accepts it as an option) or your own promisification method. For most of these modules (request included) there is already a package doing this on NPM.

@petkaantonov
Copy link
Owner

The default promisifier is for node callback convention, if you have something else, use a custom promisifier with the promisifier option https://github.com/petkaantonov/bluebird/blob/master/API.md#option-promisifier.

@thesebas
Copy link
Author

Ok, promisifier and this module looks "promising" and understand that this issue in general is not place of improvement bluebird, thx.

@spion
Copy link
Collaborator

spion commented Dec 12, 2014

I'd love it if bluebird were somehow able to handle cases like this. Unfortunately there are many variants and its pretty hard to cover all of them, so custom promisifiers is probably the best solution for now.

This could be fixed pretty easily by node's core team though. They'd just add methods like readAll(function(err, data) { }) or .ended(function(err) { ... }) to streams. Then you could promisify streams and not pass any callbacks to request:

var req = request(url);
req.readAllAsync().then(...)
// req is available for progress etc.

@benjamingr
Copy link
Collaborator

If you suggest it on io.js I'd support it

@spion
Copy link
Collaborator

spion commented Dec 12, 2014

Done here, nodejs/node#153 - came up with a basic draft proposal.

@spion
Copy link
Collaborator

spion commented Dec 15, 2014

The proposal was rejected - its not going to happen. Maybe we could build a standard replacement for request that has the same API plus additional stream methods such as collectResponse(), or a module that extends (Readable)Stream.prototype to make it actually useful

@benjamingr
Copy link
Collaborator

@spion dat edit :D

@spacemonkey92
Copy link

Tried to use promisifyAll on sign.

jwt.sign({ foo: 'bar' }, cert, { algorithm: 'RS256' }, function(token) {
  console.log(token);
});

I end up getting the token value inside the error. ( because it doesn't have any error param)
Im quite new to promises. is it just me or is it really strange ?

@phpnode
Copy link

phpnode commented Mar 4, 2016

@spacemonkey92 sounds like you're not actually using the promisified version.

@spacemonkey92
Copy link

@phpnode That was the actual method.

jwt.signAsync({ foo: 'bar' }, cert, { algorithm: 'RS256' })
      .then((token)=>{
      console.log(token);
      })
      .catch((err)=>{
      console.log(err);
      });

This is going to the error block.
gives me this
{ [Error: eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.eyJzZXNzaW9uSWQiOiJibGI1SEI1VWNqZElzNVR6c1ViR0RrOGxTWVJWQU5jWGh6QXkiLCJpYXQiOjE0NTcwOTkxNDAsImV4cCI6MTQ1NzEwMjc0MH0.V-ClT-in0dzyl1lGZ8vJh-F0Nn5fWxbXOqchhfUXOi-nsfkidN034FP-X7p-9qCl9s1WqE6_75d0WrTH5qVt-5mpKR310FJW0crf2ZDF3chXYf17_BpcqM72XotKmxi4lIaDOUlNyPxQcJeg4LX2HSKVLsGD1Aw62SsoClfGv3s]
cause: [Error: eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.eyJzZXNzaW9uSWQiOiJibGI1SEI1VWNqZElzNVR6c1ViR0RrOGxTWVJWQU5jWGh6QXkiLCJpYXQiOjE0NTcwOTkxNDAsImV4cCI6MTQ1NzEwMjc0MH0.V-ClT-in0dzyl1lGZ8vJh-F0Nn5fWxbXOqchhfUXOi-nsfkidN034FP-X7p-9qCl9s1WqE6_75d0WrTH5qVt-5mpKR310FJW0crf2ZDF3chXYf17_BpcqM72XotKmxi4lIaDOUlNyPxQcJeg4LX2HSKVLsGD1Aw62SsoClfGv3s],
isOperational: true }

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

6 participants