Q.all array of request promises, not sure how to get the results

Question

Probably an obvious answer to this but I'm not sure what way to take. request is a node module: https://github.com/request/request I fill an array of getHistory requests (with different parameters). p = [p1,p2...].

this.app.all('/api/heatmap', function(req,res) {
   // fill p here _.each(blabla, p.push(gethistory(someparams...)
   var result = [];
   function getHistory(params) {
        var options = { ...};
        var callback = function(error, response, body) {
            if(error) { //wtv 
            } else {
              // what to do with the body here ? return body ? result.push(body) ?
            }
        }

        request(options, callback);
    }

    Q.all(p).then(function() {
    });
}

So the problem here is that I when all of the request to be done , put everything in an array/object then send the whole thing to the client. How to have getHistory returning the fetched value (after the request is done ).

Hope it's clear.

Deeply related: http://stackoverflow.com/questions/22519784/how-do-i-convert-an-existing-callback-api-to-promises — Benjamin Gruenbaum, Jan 02 '16 at 17:39

Tomalak · Accepted Answer · 2016-01-02T18:02:54.727

3

The core problem here is that node.js-style callbacks and promises are not compatible. Promises emphasize on return values, node emphasizes on callbacks.

Therefore you need a sort of adapter that wraps node's callback convention properly, a process called Promisifcation. This can be done manually, but it's tedious at best and error-prone when you are not careful. Luckily, since node's conventions are well-established, it can be automated. Q has a few helpers for that, but Bluebird is quite a bit more convenient in this regard.

So the easy way to do it is to switch to Bluebird as the promise library and to use promisifyAll.

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

this.app.all('/api/heatmap', function(req, res) {
    var requests = blabla.map(function (item) {
        return request.getAsync({ /* params */ });
    });

    Promise.all(requests).then(function (responses) {
        res.send( JSON.stringify(responses) ); // whatever
    }).catch(function (error) {
        res.send( "An error ocurred: " + error ); // handle error
    });
}

edited Jan 02 '16 at 18:02

answered Jan 02 '16 at 11:42

Tomalak

332,285
67
532
628

Not sure what you mean but I found the feature I needed, it seems far simpler. Q module thought about that issue. Check my answer – François Richard Jan 02 '16 at 12:04
Not sure what you mean with "not sure what you mean". My answer is pretty clear. But congratulations, you just promisified part of the the request module manually, just as I said. It may seem simpler, but that's because you want to perceive it that way, not because it actually is. – Tomalak Jan 02 '16 at 12:07
According to the Q doc, this problem "The core problem here is that node.js-style callbacks and promises are not compatible." is addressed with the deferred feature. I've read the part about promification it's really interesting thanks. But isn't heavier/more opaque to use the promisifcation method ? If not, does it mean I should use bluebird instead of Q (since promisification is a more advanced feature ?) ? – François Richard Jan 02 '16 at 12:26
The first paragraph of my answer says the exact same thing. Yes, it's possible to do it manually, you and did it. But since `request` is by far not the only node function that uses the node-style callback convention, you will have to do it many times over, which becomes tedious (and error-prone) very quickly. My answer says that as well. I'm beginning to get the sinking feeling that you completely skipped reading the text. :-/ – Tomalak Jan 02 '16 at 12:44
That said, promisification is not an "advanced feature" that's somehow reserved for special cases. You did it yourself, after all. It has nothing to do with Bluebird - Bluebird only offers a method that *automates* the process reliably. Also, you use the `request` module instead of implementing the HTTP specification yourself, why don't you see that as "heavier/more opaque"? – Tomalak Jan 02 '16 at 12:47
=D I'm just wondering why should I use Q (or anyone) since it doesn't offer promisification method ? I'm not saying it's heavier I'm just asking, sometimes people advise overkill modules ... Would you say bluebird is more complete than Q and I should use it instead ? – François Richard Jan 02 '16 at 12:59
I wouldn't say that, if you prefer Q use it. However, I prefer Bluebird for details like this one. You can turn the API of an entire module that does not have promise support into one that has with a single line of code (`var fs = Promise.promisifyAll(require('fs'));`) - I find that pretty neat and not opaque at all. Manually fiddling `defer()` into every place that deals with node functions in a promise context however... That's a lot of brittle boilerplate. Don't mistake *"but I only need to change my original code so little"* for an overall advantage. – Tomalak Jan 02 '16 at 13:18

Tomalak · Answer 2 · 2016-01-02T17:46:19.597

FWIW, here's another answer that shows how the same would look like when done properly with Q:

// promisified request
function requestAsync(options) {
    var result = Q.defer();
    request(options, function(error, response, body) {
        if (error) {
            result.reject(error);
        } else {
            result.resolve(body);
        }
    });
    return result.promise;
}

// returns promises for heatmapVolumes
function getHistory(params) {
    return requestAsync({
        method: 'GET',
        url: 'https://api.kaiko.com/v1/charts/' + 
            encodeURIComponent(params.exchange) + '/' + 
            encodeURIComponent(params.pair) +'/history',
        qs: params.qs,
        headers: {
            "Content-Type": "application/json",
            "Accept": "application/json"
        }
    }).then(function (body) {
        return heatmapVolume(body, params.exchange, params.pair);
    }).catch(function (error) {
        // log detailed error and send less revealing message downstream
        console.error('error fetching trades', error);
        throw new Error('Something went wrong while fetching trades');
    });
}

// usage
this.app.all('/api/heatmap', function(req, res) {
    getHistory({
        exchange: "foo", pair: "bar", qs: "qux"
    }).then(function (heatmap) {
        res.send(200, heatmap);
    }).catch(function (error) {
        res.send(500, error);
    });
});

Q has `nfcall` and `nfbind` helpers - even in an older inferior library like Q, you still can and should do automatic promisificaiton — Benjamin Gruenbaum, Jan 02 '16 at 17:38
Absolutely. You'd still have to do it on a per-function-basis, though, so it would still qualify as tedious. This answer is the improved version of the OP's answer, doing what he did, minus the obvious mistakes (not sure if any non-obvious mistakes are in there, please look over it). I didn't want to post it as a pastebin link into the comments of the OP's answer. — Tomalak, Jan 02 '16 at 17:43
This would fail as in Q, `done`'s return value is `undefined` and it is used to signal the absolute end of a promise chain. — Benjamin Gruenbaum, Jan 02 '16 at 17:45

score 0 · Answer 3 · answered Jan 02 '16 at 12:05

Used Q.deferred and it worked as documented \o/

function getHistory(params) {
                var deferred = Q.defer();
                var options = {
                    method: 'GET',
                    url: 'https://api.kaiko.com/v1/charts/' + params.exchange + '/' + params.pair +'/history',
                    qs:qs,
                    headers: {
                        "Content-Type": "application/json",
                        "Accept": "application/json"
                    }
                }

                var callback = function(error, response, body) {
                    if(error) {
                        console.log('error fetching trades', error);
                        res.send(500, 'Something went wrong while fetching trades');
                    } else {
                        var body = heatmapVolume(body, params.exchange, params.pair);
                        // console.log("result!", body.exchange, body.pair);
                        result.push(body);
                        // return body;
                        deferred.resolve();
                    }
                }

                request(options, callback);

                return deferred.promise;
            }

This code has the problem that you don't call `deferred.reject()` on error and that you don't call `deferred.resolve(body)`, as it actually should be done. As I said, it's easy to make mistakes in manual promisification and these mistakes make it hard to use the potential of promises properly. — Tomalak, Jan 02 '16 at 13:26
See the other answer I have added. It shows how the code in your answer *should* look like. — Tomalak, Jan 02 '16 at 13:43

Q.all array of request promises, not sure how to get the results

3 Answers3