Reputation: 10230
I have a register controller that uses try {} catch(e) {}
mechanism to me its a pretty decent ad-hoc mechanism for error but overall is it better switching to promises as i think promises offer more granuler error handling mechanisms, so below is my code as of now in my controller.
const registerNewUser = async (req, res) => {
/*
code here for extracting info from req
*/
try {
const users_data = await users.create_new_user({
email,
crypted_pwd,
salt,
first_name,
phone_code,
});
const { id: user_id, first_name: user_first_name } = users_data;
const customer_data = await customers.create_new_customer({
first_name,
last_name,
email,
});
const {
tenant_id,
id: customer_id,
} = customer_data;
await clearCartFromRedis(merchant_canonical_name, req.token);
signUpWelcomeMailer(merchant_canonical_name, req.token, user_id);
return res.status(200).json({
success: true,
access_token: req.token,
first_name: user_first_name,
});
} catch (error) {
logger.log({ message: error.message, level: 'error' });
return res.status(500).send(error.message);
}
};
As you can see there are multiple things happening inside the try block. I.E. query to sequilize to create_new_user
and create_new_customer
, then, clearCartFromRedis
, and then signUpWelcomeMailer
. All three of these can throw different types of errors and yet i've handled them in one single catch
block inside which i have the following statement.
return res.status(500).send(error.message);
So now my question is would it have been better to handle this in a promise chain using then()
and catch()
? so say refactor the above to something like
customers.create_new_customer({
first_name,
last_name,
email,
}).then(data => {
const obj = { tenant_id: data.tenant_id, id: data.customer_id, merchant_canonical_name: data.merchant_canonical_name }
return obj
}).then((obj) => {
clearCartFromRedis()
.catch(() => { throw new Error('Could not clear cache') })
}).then(() => {
signUpWelcomeMailer(merchant_canonical_name, req.token, user_id)
.catch(() => { throw new Error('Error encountered while sending the email') })
}).catch(error => res.sendStatus(500))
So as you can see there is individual error handing as as well as a global catch block in the end. My question is in my use case is a global try ... catch
better or the promise chaining mechanism ?
P.S. there is a similar question asked HERE, but this question pertains more towards try .. catch
vs promise chaining mechanism
.
Upvotes: 1
Views: 1065
Reputation: 22783
Firstly, try... catch
is useful to catch mostly expected
errors (when you need to return some 4xxx error with additional info or even 200 with a business logic error code or something like that) rather than some unexpected errors that should lead to 500 HTTP errors. That said you need a global handler for all non-catched/unexpected errors that will log these errors and send the 500 error to a client.
global error handler
app.use(function (err, req, res, next) {
logger.error(err)
res.status(500).send('Unexpected error')
})
try/catch with an explicit transaction
const registerNewUser = async (req, res) => {
/*
code here for extracting info from req
*/
// here is some call to create a transaction in a DB
// it varies depending on a type of DB and a package you use to communicate with it
// we need to create a transaction BEFORE try/catch block
const transaction = await createTransaction();
try {
// we need to pass transaction to every method that modifies data in DB
// also depends on how to pass a transaction to underlying DB package methods
const users_data = await users.create_new_user({
email,
crypted_pwd,
salt,
first_name,
phone_code,
}, transaction);
const { id: user_id, first_name: user_first_name } = users_data;
// we need to pass transaction to every method that modifies data in DB
const customer_data = await customers.create_new_customer({
first_name,
last_name,
email,
}, transaction);
const {
tenant_id,
id: customer_id,
} = customer_data;
await clearCartFromRedis(merchant_canonical_name, req.token);
signUpWelcomeMailer(merchant_canonical_name, req.token, user_id);
// here we need to commit the transaction before we exit from this handler
await transaction.commit();
return res.status(200).json({
success: true,
access_token: req.token,
first_name: user_first_name,
});
} catch (error) {
// here we need to rollback the transaction before we exit from this handler
await transaction.rollback();
logger.log({ message: error.message, level: 'error' });
return res.status(500).send(error.message);
}
};
Secondly, in your case of several operations against some DB you clearly need to use a transaction mechanism to keep data consistent. Usually, it requires creating an explicit transaction and passing it to each query that should be executed as a one atomic operation that either executes correctly (then you need to commit a transaction) or fails (then you need to rollback a transaction). For that, you can use try...catch
to be able to commit or rollback a transaction at the end of a batch of operations.
Upvotes: 1