Should I use await inside Promise.all?

Question

I'm building express middleware to make two async calls to the database to check if the username or email are already in use. The functions return promises without a catch, as I want to keep database logic seperate from req/res/next logic, and I have centralised error handling that requires next as an argument. In my postman testing on local environment the following code works as expected and my centralised errorhandler returns the error to the client:

async checkUsernameExists(username) {
    await this.sequelize.transaction(
      async () =>
        await this.User.findOne({
          where: {
            username,
          },
        }).then(user => {
          if (user) throw new Conflict('Failed. Username already in use.');
        }),
    );
  }  

const checkDuplicateUsernameOrEmail = async (
  { body: { email, username } },
  res,
  next,
) => {

  await Promise.all([
    checkUsernameExists(username),
    checkEmailExists(email),
  ])
    .then(() => next())
    .catch(error => next(error));
};

However, as the checkExists functions are async, shouldn't they be included in Promise.all with await? Or does Promise.all do this automatically?

await Promise.all([
    await checkUsernameExists(username),
    await checkEmailExists(email),
  ])...

This leads to an unhandled promise rejection from checkUsernameExists and no response is sent back to the client.

CertainPerformance CertainPerformance · Accepted Answer · 2020-10-24T15:35:35

Should I use await inside Promise.all?

No (at least not the way you're doing it). Promise.all accepts and expects an array of Promises. Once they all resolve, or if one rejects, the Promise.all will resolve or reject. If you use await, you'll be passing an array of plain non-Promise values to Promise.all, which isn't the logic you want. If you use await, you'll also be waiting for the Promises to resolve serially, rather than in parallel, defeating the entire point of Promise.all. For example:

await Promise.all([
    await checkUsernameExists(username),
    await checkEmailExists(email),
  ])...

If checkUsernameExists takes 0.5 seconds to resolve, and checkEmailExists also takes 0.5 seconds to resolve, it will take at least 1 second for the Promise.all to resolve, because the Promises are being resolved via the await checkUsernameExistss, not by the Promise.all itself.

You should definitely do

await Promise.all([
  checkUsernameExists(username),
  checkEmailExists(email),
])

Async functions return Promises - to the Promise.all, someFnThatReturnsAPromise() is the same as somePromise. So there's absolutely nothing wrong with invoking the function and putting the resulting Promise in the array to pass to Promise.all.

Should I use await inside Promise.all?

1 Answers