1

I am building an express.js web application, and for one of the API requests I need to make multiple requests for multiple user accounts in parallel and return one object.
I tried using generators and Promise.all but I have 2 problems:

  1. I don't run in parallel for all user accounts.
  2. My code ends after the response has already returned.

Here is the code I wrote:

function getAccountsDetails(req, res) {
    let accounts = [ '1234567890', '7856239487'];
    let response = { accounts: [] };

    _.forEach(accounts, Promise.coroutine(function *(accountId) {
        let [ firstResponse, secondResponse, thirdResponse ] = yield Promise.all([
            firstRequest(accountId),
            secondRequest(accountId),
            thirdRequest(accountId)
        ]);

        let userObject = Object.assign(
            {},
            firstResponse,
            secondResponse,
            thirdResponse
        );

        response.accounts.push(userObject);
    }));

    res.json(response);
}
David Tzoor
  • 987
  • 4
  • 16
  • 33
  • What are in the firstRequest ... functions? Are those real async functions? Maybe there is some internal serialization there. – Tamas Hegedus Aug 17 '16 at 17:12
  • They are async functions which return a promise. – David Tzoor Aug 17 '16 at 17:13
  • Actually they all *are* running in parallel, the problem is that `res.json` is waiting for none of them. [You cannot use `forEach`](http://stackoverflow.com/q/37576685/1048572). – Bergi Aug 17 '16 at 17:37

1 Answers1

1

_.forEach is not aware of Promise.coroutine and it is not using the return values.

Since you're already using bluebird, you can use its promises aware helpers instead:

function getAccountsDetails(req, res) {
    let accounts = [ '1234567890', '7856239487'];
    let response = { accounts: [] };

    return Promise.map(accounts, (account) => Promise.props({ // wait for object
       firstResponse: firstRequest(accountId),
       secondResponse: secondRequest(accountId),
       thirdResponse: thirdRespones(accountId)         
    })).tap(r => res.json(r); // it's useful to still return the promise
}

And that should be the code in its entirety.

Coroutines are great, but they're useful for synchronizing asynchronous stuff - in your case you actually do want the concurrency features.

Benjamin Gruenbaum
  • 270,886
  • 87
  • 504
  • 504