I am building a basic authentication system with a node backend and wonder whether I am using the try-catch block excessively. I have this example controller:
export const getUser = async (req: Request, res: Response) => {
try {
const user = await User.findById(req.body.id);
if (!user) {
return res.status(404).send('No user found');
}
res.send(user);
} catch (err: any) {
return res.status(400).send(err.message);
}
};
And this findById
User class method:
static async findById(id: string): Promise<any> {
// try {
const collection = getCollection(USERS_COLLECTION_NAME);
const user = await collection.findOne({ _id: new mongodb.ObjectId(id) });
return user;
// } catch (err) {
// throw err;
// }
}
The controller already uses a try-catch block, so is having another try-catch block in the class method necessary? It still works after commenting out the try-catch block. How far should its usage be taken?
To give some more context, here is the User class:
import { USERS_COLLECTION_NAME } from '../utils/db';
import { getCollection } from '../utils/helpers';
import jwt from 'jsonwebtoken';
const mongodb = require('mongodb'); // do not convert to import (otherwise undefined)
// does anywhere here NOT need try catch?
export class User {
email: string;
hashedPassword: any;
dateCreated: Date;
constructor(email: string, hashedPassword: any) {
this.email = email.toLowerCase();
this.hashedPassword = hashedPassword;
this.dateCreated = new Date();
}
signToken(): string {
try {
// { ...this } overcomes error `Expected "payload" to be a plain object`
const token = jwt.sign({ ...this }, this.email, {
expiresIn: 60 * 24,
});
return token;
} catch (err) {
throw err;
}
}
async saveToDb(): Promise<any> {
try {
const collection = getCollection(USERS_COLLECTION_NAME);
const saveResult = await collection.insertOne(this);
return saveResult;
} catch (err) {
throw err;
}
}
static async delete(id: string): Promise<any> {
try {
const collection = getCollection(USERS_COLLECTION_NAME);
const user = await User.findById(id);
if (!user) throw new Error('404 - User not found');
const deleteResult = await collection.deleteOne(user);
if (!deleteResult) throw new Error('500 - Could not delete user');
return deleteResult;
} catch (err) {
throw err;
}
}
static async fetchAll(): Promise<any> {
try {
const collection = getCollection(USERS_COLLECTION_NAME);
const users = await collection.find().toArray(); // @todo: may need to optimise as it gets ALL users - what if a million users existed?
return users;
} catch (err) {
throw err;
}
}
static async findByEmail(email: string): Promise<any> {
try {
const collection = getCollection(USERS_COLLECTION_NAME);
const user = await collection.findOne({ email });
return user;
} catch (err) {
throw err;
}
}
static async findById(id: string): Promise<any> {
// try {
const collection = getCollection(USERS_COLLECTION_NAME);
const user = await collection.findOne({ _id: new mongodb.ObjectId(id) });
return user;
// } catch (err) {
// throw err;
// }
}
}
And the controllers:
import { Request, Response } from 'express';
import { User } from '../models/auth';
import bcrypt from 'bcrypt';
import { USERS_COLLECTION_NAME } from '../utils/db';
import { getCollection } from '../utils/helpers';
const mongodb = require('mongodb'); // do not convert to import (otherwise undefined)
export const createUser = async (req: Request, res: Response) => {
const { email, password } = req.body;
const existingUser = await User.findByEmail(email);
if (existingUser) {
return res.status(409).send('User already exists. Please log in.');
}
const hashedPassword = await bcrypt.hash(password, 10);
const newUser = new User(email, hashedPassword);
const saveToDb = await newUser.saveToDb();
if (!saveToDb) {
return res.status(500).send('Could not insert user into the database.');
}
const token = newUser.signToken();
res.status(201).json({ token, id: saveToDb.insertedId });
};
// only updates email for now...
export const updateUser = async (req: Request, res: Response) => {
const collection = getCollection(USERS_COLLECTION_NAME);
try {
const updateResult = await collection.updateOne(
{
_id: new mongodb.ObjectId(req.body.id), // new mongodb.ObjectId needed, otherwide null; converts to BSON
},
{
$set: {
email: req.body.email,
},
}
);
if (!updateResult.modifiedCount) {
return res.status(500).send('Could not update user.');
}
res.send({ updateResult });
} catch (err: any) {
return res.status(400).send(err.message);
}
};
export const deleteUser = async (req: Request, res: Response) => {
const { id } = req.body;
try {
const deleteUserResult = await User.delete(id);
res.send({ deleteUserResult });
} catch (err: any) {
return res.status(500).send(err.message); // would prefer to set status code from err as it's not always 500! e.g. the delete class method can throw a 404 or 500 error
}
};
export const getUser = async (req: Request, res: Response) => {
try {
const user = await User.findById(req.body.id);
if (!user) {
return res.status(404).send('No user found');
}
res.send(user);
} catch (err: any) {
return res.status(400).send(err.message);
}
};
export const getUsers = async (req: Request, res: Response) => {
const users = await User.fetchAll();
if (!users) {
return res.status(404).send('No users found');
}
res.send(users);
};
export const logUserIn = async (req: Request, res: Response) => {
const { email, password } = req.body;
const user = await User.findByEmail(email);
if (!user) {
return res.status(404).send('User not found. Please create an account.');
}
const correctPassword = await bcrypt.compare(password, user.hashedPassword);
if (!correctPassword) {
return res.status(403).send('Wrong password for this account.');
}
// assign object methods to the user instance as objects retrieved from db don't have methods
Object.setPrototypeOf(user, User.prototype);
const token = user.signToken();
return res.json({ token, id: user._id });
};
Any other tips or suggestions would also be greatly appreciated, thank you!