CODEWITHSUNDEEP
javascriptnode.jsexpressapi-design
luketurnerdev
luketurnerdev

Reputation: 31

Is there a better way to handle errors in express?

I'm building an API in express after some time away from it. This app helps users track their guitar routines. Here is an example of a DELETE function in my controller and the possible exceptions that can be thrown:

 deleteOneRoutine = async (userId, routineId) => {
  // If the given routineId is found in the DB 
  // AND the userId matches, delete it from DB

  const selectedRoutine = await Routine.findOne({_id: routineId});
  if (!selectedRoutine) {return 400};

  if (selectedRoutine.userId != userId) {return 401};

  const deleted = await selectedRoutine.remove();
  if (!deleted) {return 500};

  return deleted;
}

And here is the route that receives those error codes:

  routineRouter.delete('/:routineId', async (req, res) => {
   const userId = req.params.userId;
   const routineId = req.params.routineId;
   const deleted = await routineController.deleteOneRoutine(userId, routineId);
   if (deleted === 400) {res.status(400).send('Requested routine could not be found.')}
   else if (deleted === 401) {res.status(401).send('Unauthorized user.')}
   else if (deleted === 500) {res.status(500).send('Server error. Could not delete routine.')}
   else {res.status(200).send(`Successfully deleted routine with ID ${routineId}`)}  
});

I'm wondering if there is a better way to handle this instead of manually passing back the error codes from the controller. Any help would be appreciated!

Upvotes: 3

Views: 43

Answers (2)

Codebling
Codebling

Reputation: 11382

It would be easier to throw an error and add an error handler in Express.

router.use((err, req, res, next) => {
  res.status(500).send(err.message);
});

router.get('/path', async (req, res) => {
  throw new Error('Can't delete!');
});

I think the error middleware has to come first, but I'd have to check.

If you need specific error types you can subclass Error; if you want to be able to pass a code, you could add a property on to the Error (though I would recommend avoiding mixing routing logic (e.g. setting specific HTTP codes) with business logic (e.g. your deleteOneRoutine function))

Upvotes: 0

CertainPerformance
CertainPerformance

Reputation: 370809

I'm not so happy about the current typing of the return value - I think returning an object with either an error in an error property or the routine ID would make more sense. But a better way IMO would be to pass down the res so that deleteOneRoutine can do it itself. Also make a helper function to keep things DRY, and don't forget to catch possible exceptions thrown by the asynchronous functions:

deleteOneRoutine = async (userId, routineId, res) => {
  const send = (code, message) => res.status(code).send(message);
  const serverError = () => send(500, 'Server error. Could not delete routine.');
  try {
    const selectedRoutine = await Routine.findOne({_id: routineId});
    if (!selectedRoutine) return send(400, 'Requested routine could not be found.');
    if (selectedRoutine.userId !== userId) return send(401, 'Unauthorized user.');
    const deleted = await selectedRoutine.remove();
    if (!deleted) return serverError();
    send(200, `Successfully deleted routine with ID ${deleted}`);
  } catch(e) {
    serverError();
  }
}

routineRouter.delete('/:routineId', (req, res) => {
  const { userId, routineId } = req.params;
  routineController.deleteOneRoutine(userId, routineId, res);
});

Upvotes: 1

Related Questions