I wanted to double-check my thinking on using TypeScript in ExpressJS. Here's my abridged code in question:
/**
* Verifies that an email address exists in the DB.
* If so, sends them an email to recover their password.
*
* @param {Request} req
* @param {Response} res
*/
const sendPasswordRecoveryEmail = (req: Request, res: Response) => {
const email = req.body.email
const checkForEmailAddress = async (email: string) => {
return await pool.query(`SELECT password FROM users WHERE email = $1`, [email]);
};
const updatePasswordFields = async (password: string, newPassword: string, email: string) => {
return await pool.query(`UPDATE users SET last_password = $1, password = $2 WHERE email = $3`, [password, newPassword, email]);
};
const passwordRecoveryProcess = async (email: string) => {
try {
res.status(200).send({ status: true, message: 'Password recovery process initiated.' });
const results = await checkForEmailAddress(email);
if (results.rows.length === 1) {
const existingPassword = results.rows[0].password;
const newTempPassword = generateRandomPassword();
await updatePasswordFields(existingPassword, newTempPassword, email);
const body = `...`;
emailService({email, subject: 'MySite Password Recovery', body});
}
} catch (error) {
console.error('Error', error);
}
}
passwordRecoveryProcess(email); // Initiate async Password Recovery Process
};
With this line of code:
const results = await checkForEmailAddress(email);
I'm using await
before calling the function. Then within the function itself I'm also using async
/await
. Is this the correct practice or is there some redundancy with it?
CodePudding user response:
Nested await
s inside functions calling other async functions / Promises is completely normal, especially when you're dealing with a library or are trying to abstract away some of the logic.
That said, in this particular situation, where everything is defined inside a single relatively short function, and your extra functions are composed of only a single line and only get called once, I'm not seeing how they're helping anything. There is no repetitive logic to abstract away. You may as well do
const sendPasswordRecoveryEmail = (req: Request, res: Response) => {
const { email } = req.body;
try {
res.status(200).send({ status: true, message: 'Password recovery process initiated.' });
const emailResult = await pool.query(`SELECT password FROM users WHERE email = $1`, [email]);
if (results.rows.length === 1) {
const existingPassword = emailResult.rows[0].password;
const newTempPassword = generateRandomPassword();
await pool.query(`UPDATE users SET last_password = $1, password = $2 WHERE email = $3`, [existingPassword, newTempPassword, email]);
const body = `...`;
emailService({email, subject: 'MySite Password Recovery', body});
}
} catch (error) {
console.error('Error', error);
}
};
That said, what you're doing now isn't forbidden - it just looks a little bit odd (though there's no need to await
a Promise you're returning immediately, if you aren't inside a try
block - just return the Promise, don't await
it).
But you might consider only sending a response to the user after successfully getting to the email service, something like:
const sendPasswordRecoveryEmail = (req: Request, res: Response) => {
const { email } = req.body;
try {
const emailResult = await pool.query(`SELECT password FROM users WHERE email = $1`, [email]);
if (results.rows.length !== 1) {
res.status(401).send({ status: false, message: 'Email not found' });
return;
}
const existingPassword = emailResult.rows[0].password;
const newTempPassword = generateRandomPassword();
await pool.query(`UPDATE users SET last_password = $1, password = $2 WHERE email = $3`, [existingPassword, newTempPassword, email]);
const body = `...`;
// does the below need an `await`, perhaps?
emailService({email, subject: 'MySite Password Recovery', body});
res.status(200).send({ status: true, message: 'Password recovery process initiated.' });
} catch (error) {
console.error('Error', error);
res.status(500).send({ status: false, message: 'Unexpected server error' });
}
};