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.
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 checkUsernameExists
s, 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
.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With