Try catch block best practices

Hello, consider this route code:
import express from "express";
import { ObjectId } from "mongodb";
import { db } from "../server.js";

const orderRouter = express.Router();

// Add order to card
orderRouter.post("", async (req, res) => {
// get request value inside req.body
const id = req.body.id;

// Retrieve lesson details
const product = await db
.collection("products")
.find({ _id: new ObjectId(id) })
.toArray();

const [order] = product;

if (order.spaces > 0) {
let [newOrder] = product;

await db.collection("orders").insertOne({
src: newOrder.src,
subject: newOrder.subject,
location: newOrder.location,
price: newOrder.price,
});

newOrder = {
src: newOrder.src,
subject: newOrder.subject,
location: newOrder.location,
price: newOrder.price,
};

res
.status(200)
.json({ message: "Order successfully created !", order: newOrder });
} else {
res
.status(500)
.json({ message: "Number of spaces depleted, cannot add to cart !" });
}
});

export { orderRouter };
import express from "express";
import { ObjectId } from "mongodb";
import { db } from "../server.js";

const orderRouter = express.Router();

// Add order to card
orderRouter.post("", async (req, res) => {
// get request value inside req.body
const id = req.body.id;

// Retrieve lesson details
const product = await db
.collection("products")
.find({ _id: new ObjectId(id) })
.toArray();

const [order] = product;

if (order.spaces > 0) {
let [newOrder] = product;

await db.collection("orders").insertOne({
src: newOrder.src,
subject: newOrder.subject,
location: newOrder.location,
price: newOrder.price,
});

newOrder = {
src: newOrder.src,
subject: newOrder.subject,
location: newOrder.location,
price: newOrder.price,
};

res
.status(200)
.json({ message: "Order successfully created !", order: newOrder });
} else {
res
.status(500)
.json({ message: "Number of spaces depleted, cannot add to cart !" });
}
});

export { orderRouter };
It works fine but I have the bad habit of not writing "robust" code, like using try catch when it comes to network request like fetching data from a database. I was wondering if anyone can share their experience about when should I use try catch block and how can I make it become natural to me, like when I code, I don't really think about it like in the code above, I didn't really bother about it :c.
10 Replies
13eck
13eck5d ago
You should be using try/catch any time the operation could fail and throw an error. Unfortunately there's no master list of APIs that can throw an error, so you just learn by failing…or you read the MDN docs for each and every method you use to see if it can throw an error. Another thing you can do is use Promises instead of async/await. As long as you have a .catch() method somewhere you're handling any/all errors that might get thrown! For example:
// prior code omitted for brevity
// Add order to card
// NOTE: I added a `/` here to denote
// that this is the default/home route
orderRouter.post("/", (req, res) => {
// get request value inside req.body
const id = req.body.id;

// Retrieve lesson details
db.collection("products")
.find({ _id: new ObjectId(id) })
.toArray().then( ([order]) => order)
// By throwing an error we bypass all the following
// `.then()` chains and go straight to the `.catch()`
.then( (order) => order.spaces > 0 ? order : throw new Error("Number of spaces depleted, cannot add to cart !"))
.then( (order) => {
const newOrder = { src: order.src, subject: order.subject, location: order.location, price: order.price };
// I'm assuming that db.collection().insertOne()
// returns the object that is being inserted
return db.collection("orders").insertOne(newOrder);
})
.then( (newOrder) => res.status(200).json({ message: "Order successfully created !", order: newOrder })
// any errors thrown or rejected promises are directed to the
// `catch()` method. Otherwise it's ignored
.catch( (error) => res.status(500).json({message: error.message}))
});
// prior code omitted for brevity
// Add order to card
// NOTE: I added a `/` here to denote
// that this is the default/home route
orderRouter.post("/", (req, res) => {
// get request value inside req.body
const id = req.body.id;

// Retrieve lesson details
db.collection("products")
.find({ _id: new ObjectId(id) })
.toArray().then( ([order]) => order)
// By throwing an error we bypass all the following
// `.then()` chains and go straight to the `.catch()`
.then( (order) => order.spaces > 0 ? order : throw new Error("Number of spaces depleted, cannot add to cart !"))
.then( (order) => {
const newOrder = { src: order.src, subject: order.subject, location: order.location, price: order.price };
// I'm assuming that db.collection().insertOne()
// returns the object that is being inserted
return db.collection("orders").insertOne(newOrder);
})
.then( (newOrder) => res.status(200).json({ message: "Order successfully created !", order: newOrder })
// any errors thrown or rejected promises are directed to the
// `catch()` method. Otherwise it's ignored
.catch( (error) => res.status(500).json({message: error.message}))
});
I much prefer this method to using async/await because it is very obvious what is being done when/where. And having the .catch() at the end means we all know where the errors are handled, if any. Also, you really need to standardize your code. In your code snippet here you're using the modern (and best-practice) import declaration while in other code snippets you've posted you use the older (and not as good, IMO) require() function. Choose one! -# pro tip: choose import and never look back
Faker
FakerOP5d ago
yeah noted, I will have to get use to that, speaking of using promises instead of async and await, yeah you are right, I think it's much more obvious, I prefer to use async ans await because of syntax sugar but at some point it becomes confusing to what we want to convey yeahh, I use ES6 in my projects normally, just when I see code here and there and just copy and paste it's old version :c
13eck
13eck5d ago
Yeah, it’s totally a personal preference thing, but one you start nesting async functions things can get difficult to read.
Faker
FakerOP5d ago
yep agreed, so far I was using simple lines of await and async but I realised it might not be that easy to read :c
13eck
13eck5d ago
You can make it even easier to read if you take the anonymous functions out of the then/catch and make named functions. Then you can do something like this:
asyncFunction
.then(doThingOne)
.then(doSedondThing)
.catch(somethingWentWrong);
asyncFunction
.then(doThingOne)
.then(doSedondThing)
.catch(somethingWentWrong);
Faker
FakerOP5d ago
yeppp true will try to use that from time to time just to get use to it thanks !
Choo♚𝕂𝕚𝕟𝕘
If you are not thinking about how it can fail, then you aren't testing properly, or maybe not at all. Note that I am referring to formal testing like with Jest or Vitest and not just trying out some code to see if it does what you expect.
Faker
FakerOP5d ago
yeah, tbh I haven't really do some unit testing or testing when it comes to JS, I did some unit testing back then in C# but it was just an introduction, haven't really dive into testing as such
StefanH
StefanH5d ago
then, catch and async, await do the same thing, they just look different. Different pieces of code might make more sense to be written one way or the other I'd also like to note that javascript's inability to convey which errors can be returned by a function is a failure of the language. I never throw errors in my code. I prefer to use the type system to return an error type if possible. Errors can catch you off guard in really dumb ways that don't feel like the programmer's fault to me. Like fs.mkdir throwing an error when the directory already exists... Javascript only tells you which errors are thrown from a function by reading documentation which is not a success of the language and typescript didn't really improve on that either as far as i know If you use try catch, also try to make the try block as small as possible and limit it to one erronious function / error type if possible. I've seen awful code that just wraps a 100 line functions in a single try block. Catch each error one by one. If you think that's unnecessarily verbose, you're right! But that's a failure of the try catch syntax itself. I really don't like it. Prefer to just return error types like undefined in your own code and isolate try catch to the native apis that still use it
Faker
FakerOP4d ago
If you use try catch, also try to make the try block as small as possible and limit it to one erronious function / error type if possible. I've seen awful code that just wraps a 100 line functions in a single try block.
xD that's a thing I can relate, I have that bad habit too :c

Did you find this page helpful?