0
\$\begingroup\$

I'm confused why you need to use try catch. I made a simply utils like this, and use it on my models' callback.

in my utils/error.js

module.exports.handleError = (err, errMsg, res) => {
    res.json({
        success: false,
        msg: `${errMsg}. ${err}` 
    })
}

Then I have this in my router controller

router.get('/tasks', (req, res) => {

    Task.getTasks((err, resp) => {
        if(err){
            return handleError(err, 'Failed to get tasks', res)
        }

        res.json({
            success: true,
            msg: {tasks: resp}
        })
    })
})

What can be improved? And what is the drawback of using this approach?

\$\endgroup\$
1
\$\begingroup\$

My main problems with your approach are:

  1. You have to remember to do this error handling. If you forget, you'll get a silent failure.

  2. You have to make every middleware aware of your error handling.

The first one can't really be solved by Express. Due to the way that Express works, you'll always have to do some variant of:

if (error) {
  return doSomething(error)
}

I prefer Koa as a result because Koa uses async/await, which translates to Promises under the hood; errors that occur in a Promise will propagate correctly, regardless of if you remember to handle them or not.

As for the second result, this kinda invalidates the purpose of middleware. Middleware should only know what it does; you can easily leave error handling til later on in the chain by modifying your middleware to pass the error argument to the next middleware in the chain..

router.get('/tasks', (request, response, next) => {
  Tasks.getTasks((error, result) => {
    if (error) {
      return next(error)
    }

    ....
  })
})

And then adding the following as the final middleware in Express:

app.use(function errorHandler(error, request, response, next) {
  return handleError(error, request, response)
})

As long as all other middlewares pass the error to the next in the chain, this will work 'automagically' and you can swap out your implementation of error handling without any middleware having to change.

As a side note, I'd highly recommend making your application use Promises and async/await (Node 8 is now LTS which supports this so there's no reason not to). You can still use express if you want to, and it would look something like this instead:

router.get('/tasks', async (req, res, next) => {
  try {
    res.json({
      success: true,
      msg: {
        tasks: await getTasks()
    })
  } catch (error) {
    next(error)
  }
})

If you used Koa you wouldn't need the try/catch.

I'd also recommend not having the success flag: Use HTTP status codes to indicate success or failure.

\$\endgroup\$
  • \$\begingroup\$ +1 for the explanation on error handling middleware. Excellent answer. \$\endgroup\$ – omgimanerd Jul 10 '17 at 20:33
0
\$\begingroup\$

When you use the traditional Node-style error-first callbacks then you often don't need try/catch - except for functions like JSON.parse() or JSON.stringify() that always have to be in try/catch blocks - see those answers for more info why:

But if you prefer using try/catch for all error handling (both for synchronous and asynchronous errors), then you can use async/await. Do do that you'll need to use functions that return promises (including functions declared with the async keyword) instead of functions that take callbacks. Many modules like Bluebird offer a way to promissify your existing functions to achieve that.

See those answers for some examples:

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.