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.

• javascript
• node.js
• express

2 Answers 2

1. Yes, you can throw errors from service layer, it is good practice to catch errors with try/catch block in controller

2. 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!')
     })
   

3. 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