I am writing an application in Express.js with a separate controller layer and a service layer. Here is my current code:
user.service.js
exports.registerUser = async function (email, password) {
const hash = await bcrypt.hash(password, 10);
const countUser = await User.countDocuments({email: email});
if(countUser > 0) {
throw ({ status: 409, code: 'USER_ALREADY_EXISTS', message: 'This e-mail address is already taken.' });
}
const user = new User({
email: email,
password: hash
});
return await user.save();
};
exports.loginUser = async function (email, password) {
const user = await User.findOne({ email: email });
const countUser = await User.countDocuments({email: email});
if(countUser === 0) {
throw ({ status: 404, code: 'USER_NOT_EXISTS', message: 'E-mail address does not exist.' });
}
const validPassword = await bcryptpare(password, user.password);
if (validPassword) {
const token = jwt.sign({ email: user.email, userId: user._id }, process.env.JWT_KEY, { expiresIn: "10s" });
return {
token: token,
expiresIn: 3600,
userId: user._id
}
} else {
throw ({ status: 401, code: 'LOGIN_INVALID', message: 'Invalid authentication credentials.' });
}
};
user.controller.js
exports.userRegister = async function (req, res, next) {
try {
const user = await UserService.registerUser(req.body.email, req.body.password);
res.status(201).json({ data: user });
} catch (e) {
if(!e.status) {
res.status(500).json( { error: { code: 'UNKNOWN_ERROR', message: 'An unknown error occurred.' } });
} else {
res.status(e.status).json( { error: { code: e.code, message: e.message } });
}
}
}
exports.userLogin = async function (req, res, next) {
try {
const user = await UserService.loginUser(req.body.email, req.body.password);
res.status(200).json({ data: user });
} catch (e) {
if(!e.status) {
res.status(500).json( { error: { code: 'UNKNOWN_ERROR', message: 'An unknown error occurred.' } });
} else {
res.status(e.status).json( { error: { code: e.code, message: e.message } });
}
}
}
The code works, but requires some corrections. I have a problem with error handling. I want to handle only some errors. If another error has occurred, the 500 Internal Server Error will be returned.
1) Can I use "throw" object from the service layer? Is this a good practice?
2) How to avoid duplication of this code in each controller:
if(!e.status) {
res.status(500).json( { error: { code: 'UNKNOWN_ERROR', message: 'An unknown error occurred.' } });
} else {
res.status(e.status).json( { error: { code: e.code, message: e.message } });
}
3) Does the code require other corrections? I'm just learning Node.js and I want to write the rest of the application well.
I am writing an application in Express.js with a separate controller layer and a service layer. Here is my current code:
user.service.js
exports.registerUser = async function (email, password) {
const hash = await bcrypt.hash(password, 10);
const countUser = await User.countDocuments({email: email});
if(countUser > 0) {
throw ({ status: 409, code: 'USER_ALREADY_EXISTS', message: 'This e-mail address is already taken.' });
}
const user = new User({
email: email,
password: hash
});
return await user.save();
};
exports.loginUser = async function (email, password) {
const user = await User.findOne({ email: email });
const countUser = await User.countDocuments({email: email});
if(countUser === 0) {
throw ({ status: 404, code: 'USER_NOT_EXISTS', message: 'E-mail address does not exist.' });
}
const validPassword = await bcrypt.pare(password, user.password);
if (validPassword) {
const token = jwt.sign({ email: user.email, userId: user._id }, process.env.JWT_KEY, { expiresIn: "10s" });
return {
token: token,
expiresIn: 3600,
userId: user._id
}
} else {
throw ({ status: 401, code: 'LOGIN_INVALID', message: 'Invalid authentication credentials.' });
}
};
user.controller.js
exports.userRegister = async function (req, res, next) {
try {
const user = await UserService.registerUser(req.body.email, req.body.password);
res.status(201).json({ data: user });
} catch (e) {
if(!e.status) {
res.status(500).json( { error: { code: 'UNKNOWN_ERROR', message: 'An unknown error occurred.' } });
} else {
res.status(e.status).json( { error: { code: e.code, message: e.message } });
}
}
}
exports.userLogin = async function (req, res, next) {
try {
const user = await UserService.loginUser(req.body.email, req.body.password);
res.status(200).json({ data: user });
} catch (e) {
if(!e.status) {
res.status(500).json( { error: { code: 'UNKNOWN_ERROR', message: 'An unknown error occurred.' } });
} else {
res.status(e.status).json( { error: { code: e.code, message: e.message } });
}
}
}
The code works, but requires some corrections. I have a problem with error handling. I want to handle only some errors. If another error has occurred, the 500 Internal Server Error will be returned.
1) Can I use "throw" object from the service layer? Is this a good practice?
2) How to avoid duplication of this code in each controller:
if(!e.status) {
res.status(500).json( { error: { code: 'UNKNOWN_ERROR', message: 'An unknown error occurred.' } });
} else {
res.status(e.status).json( { error: { code: e.code, message: e.message } });
}
3) Does the code require other corrections? I'm just learning Node.js and I want to write the rest of the application well.
Share Improve this question asked Nov 30, 2019 at 16:03 napster993napster993 1654 silver badges10 bronze badges2 Answers
Reset to default 7Yes, you can throw errors from service layer, it is good practice to catch errors with try/catch block in controller
I handle this with a custom error middleware, just use a next function in a catch block.
catch (e) { next(e) }
Example of error middleware (for more info check docs, fill free to move a middleware to file)
app.use(function (err, req, res, next) { // err is error from next(e) function // you can do all error processing here, logging, parsing error messages, etc... res.status(500).send('Something broke!') })
From my point of view it looks good. If you looking for some best practice and tools, try eslint (with AirBnb config for example) for linting, dotenv for a environment variables management, also check Node.js Best Practice
i want to give you an example:
this code in your controller
findCar(idCar)
} catch (error) {
switch (error.message) {
case ErrorConstants.ELEMENT_NOT_FOUND('LISTING'): {
return {
response: {
message: ErrorMessages.ELEMENT_NOT_FOUND_MESSAGE('LISTING'),
},
statusCode,
}
}
default: {
return {
response: {
message: ErrorMessages.UNKNOWN_ERROR_MESSAGE,
},
statusCode,
}
}
}
}
and this code in your service
findCar: async listingId => {
try {
if (some condition) {
throw new Error(ErrorConstants.ELEMENT_NOT_FOUND('LISTING'))
}
return { ... }
} catch (error) {
console.error(error.message)
throw new Error(ErrorConstants.UNKNOWN_ERROR)
}
},
controller is going to catch the service's errors