How to make run nested asynchronous methods synchronously?

Question

How do I wrap this routine inside a Promise so that I only resolve when I get all the data?

var accounts = [];
getAccounts(userId, accs => {
    accs.forEach(acc => {
        getAccountTx(acc.id, tx => {
            accounts.push({
                'id': acc.id,
                'tx': tx
            });
        });
    })
});

EDIT: Any issues if I do it like this?

function getAccountsAllAtOnce() {

    var accounts = [];
    var required = 0;
    var done = 0;

    getAccounts(userId, accs => {
        required = accs.length;
        accs.forEach(acc => {
            getAccountTx(acc.id, tx => {
                accounts.push({
                    'id': acc.id,
                    'tx': tx
                });

                done = done + 1;
            });
        })
    }); 

    while(done < required) {
        // wait
    }

    return accounts;
}

see promise all : developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/… wrap your for each with the promisAll and make sure you return a promise in each iteration of your "forEach' — Robert Rowntree
– Robert Rowntree, Commented Dec 3, 2017 at 5:36
You're populating an array called accounts with a set of objects, each of which has an account ID and a transaction. Depends on what is consuming this data, but it would be more readable for accounts to be an array of account objects, each one containing a single id value and an array of tx. And I'd expect it to be named accountTransactions instead of accounts. — stone
– stone, Commented Dec 3, 2017 at 6:08
You cannot run asynchronous functions synchronously (so that they return their results immediately). All you can do is run them sequentially. — Bergi
– Bergi, Commented Dec 3, 2017 at 15:36

Bloomca · Accepted Answer · 2017-12-03 14:18:09Z

Let's put this routine into a separate function, so it is easier to re-use it later. This function should return a promise, which will be resolved with array of accounts (also I'll modify your code as small as possible):

function getAccountsWithTx(userId) {
  return new Promise((resolve, reject) => {
    var accounts = [];
    getAccounts(userId, accs => {
      accs.forEach(acc => {
        getAccountTx(acc.id, tx => {
          accounts.push({
            'id': acc.id,
            'tx': tx
          });
          // resolve after we fetched all accounts
          if (accs.length === accounts.length) {
            resolve(accounts);
          }
        });
      });
    });
  });
}

The single difference is just returning a promise and resolving after all accounts were fetched. However, callbacks tend your codebase to have this "callback hell" style, when you have a lot of nested callbacks, and it makes it hard to reason about it. You can workaround it using good discipline, but you can simplify it greatly switching to returning promises from all async functions. For example your func will look like the following:

function getAccountsWithTx(userId) {
  getAccounts(userId)
    .then(accs => {
       const transformTx = acc => getAccountTx(acc.id)
         .then(tx => ({ tx, id: acc.id }));

       return Promise.all(accs.map(transformTx));
    });
}

Both of them are absolutely equivalent, and there are plently of libraries to "promisify" your current callback-style functions (for example, bluebird or even native Node util.promisify). Also, with new async/await syntax it becomes even easier, because it allows to think in sync flow:

async function getAccountsWithTx(userId) {
  const accs = await getUserAccounts(userId);

  const transformTx = async (acc) => {
    const tx = getAccountTx(acc.id);

    return { tx, id: acc.id };
  };

  return Promise.all(accs.map(transformTx));
}

As you can see, we eliminate any nesting! It makes reasoning about code much easier, because you can read code as it will be actually executed. However, all these three options are equivalent, so it is up to you, what makes the most sense in your project and environment.

How would you use util.promisify to wrap the OP's getAccounts() and getAccountTx()? Looking at the docs you linked, and it's pretty clear how to use util.promisify for error-first callbacks; but for the docs example, wrapping a function with a non-error-first callback using util.promisify ("custom promisified functions") looks like it adds more code, complexity and unreadability than just wrapping the call in a new Promise() would. I must be missing something.

stone · Accepted Answer · 2017-12-03 06:34:59Z

1

I'd split every step into its own function, and return a promise or promise array from each one. For example, getAccounts becomes:

function getAccountsAndReturnPromise(userId) {
    return new Promise((resolve, reject) => {
        getAccounts(userId, accounts => {
             return resolve(accounts);
        });
    });
};

And getAccountTx resolves to an array of { id, tx } objects:

function getAccountTransactionsAndReturnPromise(accountId) {
    return new Promise((resolve, reject) => {
        getAccountTx(account.id, (transactions) => {
             var accountWithTransactions = {
                 id: account.id,
                 transactions
             }; 
             return resolve(accountWithTransactions);
        });
    });
};

Then you can use Promise.all() and map() to resolve the last step to an array of values in the format you desire:

function getDataForUser(userId) {
  return getAccountsAndReturnPromise(userId)
  .then(accounts=>{
    var accountTransactionPromises = accounts.map(account => 
      getAccountTransactionsAndReturnPromise(account.id)
    );
    return Promise.all(accountTransactionPromises);
  })
  .then(allAccountsWithTransactions => {
    return allAccountsWithTransactions.map(account =>{ 
        return { 
            id: account.id, 
            tx: tx
        }
    });
  });
}

edited Dec 3, 2017 at 6:34

answered Dec 3, 2017 at 6:22

stone

8,7025 gold badges58 silver badges66 bronze badges

2 Comments

Jake Over a year ago

I also managed to get this, but is this is most elegant way to do it? It seems like a lot of work just to wait for all the data to complete.

stone Over a year ago

You're actually doing some complex stuff here: an async call followed by an array of async calls, followed by data manipulation to get it into the desired format. I'm sure there are more elegant ways to do it - I only spent 20 minutes on it, after all - but they would be variations on the same concepts.

Collectives™ on Stack Overflow

How to make run nested asynchronous methods synchronously?

2 Answers 2

1 Comment

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related