0

I am trying to make two HTTP requests to retrieve data, each of which has a callback function. Only after both callback functions have completed do I want to run the final bit of code. Maybe it's my unfamiliarity with promises, but I can't seem to get this to work. I've only gotten it to run the final thenable code either immediately or never.

var p1 = getStatus(account1, currency, processStatus)
var p2 = getStatus(account2, currency, processStatus)

Promise.all([p1, p2]).then(function() {
    // evaluate complete status
})

getStatus is a coffeescript function that makes an HTTP request, and once the data is retrieved, it calls the provided callback function, which is the third parameter.

getStatus: (acctId, curr, callback) =>
    options = {url: url, account: acctId, currency: curr}
    new Promise => request options, (err, resp, body) =>
        if !err
            return Promise.resolve callback(null, acctId, curr, JSON.parse(body))

processStatus is a JavaScript function that crunches through the retrieved data.

module.exports.processStatus = function(err, acctId, curr, status) {
    status.forEach(function(s) {
        // ....
    })
    return Promise.resolve(true)
})

How do I change this to make the evaluate complete status code execute only after both processStatus callbacks have completed?

greymatter
  • 840
  • 1
  • 10
  • 25
  • 2
    Your `processStatus` doesn't appear to be asynchronous. Why does it return a promise? – Bergi Jun 23 '16 at 01:38
  • I thought it needed to return a promise since it is the callback function for getStatus. I couldn't see another way to have Promise.all() wait until the two callbacks completed. Would your answer below work, even if processStatus did not return a promise? – greymatter Jun 23 '16 at 02:04
  • 1
    No, the promise of `getStatus` and its `callback` parameter have nothing to do with each other. Yes, my answer works with any callback functions returning anything. – Bergi Jun 23 '16 at 02:51

2 Answers2

3

You seem to be confusing the static Promise.resolve method with the resolve function that is passed into the executor callback of the new Promise constructor. Correct would be

getStatus: (acctId, curr, callback) =>
    options = {url: url, account: acctId, currency: curr}
    new Promise (resolve, reject) => request options, (err, resp, body) =>
#               ^^^^^^^^^^^^^^^^^
        if !err
            resolve callback(null, acctId, curr, JSON.parse(body))
        else
            reject err

But actually even better would be

getStatus: (acctId, curr, callback) =>
    options = {url: url, account: acctId, currency: curr}
    new Promise (resolve, reject) =>
        request options, (err, resp, body) =>
            if !err
                resolve body
            else
                reject err
    .then JSON.parse
    .then (status) => callback null, acctId, curr, status
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • Is the next-to-last line in the second codeblock missing something? How does the result of JSON.parse end up in the variable status? – greymatter Jun 23 '16 at 02:06
  • @greymatter: Nothing missing. `JSON.parse` is directly used as the callback here. [It's magic!](http://stackoverflow.com/a/22562045/1048572) (er, I mean, a functor) – Bergi Jun 23 '16 at 02:46
0

Try using reflect. Refer to the pseudo code.

    // Extend the ability of promise.all() to wait until all promises are resolved/rejected before returning.
    // By default, promise.all() will return if any 1 of the promise pass or fails.
    var reflect = function reflect(promise){
        return promise.then(function(/*resolve return value=*/v){ return { v:v, status: "resolved" }},
            function(/*rejection error=*/e){ return { e:e, status: "rejected" }});
    };


    var promises = [
        new Promise(function(resolve, reject) {
            resolve();
        }),
        new Promise(function(resolve, reject) {
            resolve();
        })
    ];

    Promise.all(promises.map(reflect)).done(function() { resolve(); });
Samuel Toh
  • 18,006
  • 3
  • 24
  • 39
  • The declaration of your `promises` consists of syntax errors and doesn't appear to make any sense – Bergi Jun 23 '16 at 01:44
  • Note: it is just pseudo code. The main idea is about the reflect not the promises. – Samuel Toh Jun 23 '16 at 01:52
  • 1
    @Bergi I have refractored the promise part. It should now make more sense. Thanks for pointing out again. Apologise for the confusion. – Samuel Toh Jun 23 '16 at 01:58
  • From what I see at http://bluebirdjs.com/docs/api/promise.all.html, what you said seems accurate for the reject case but not for the resolve case (all must pass). Am I misunderstanding? Anyway, thanks for letting me know about reflect(). – greymatter Jun 23 '16 at 02:46
  • @greymatter I just did an experiment and you are correct. So my suggest here is invalid for this case. Please feel free to down vote it. Reflect should only be used if one would like to wait for all promises to finish no matter what. That is, even when one got hit by an error it still waits for other promises. – Samuel Toh Jun 23 '16 at 03:47