My main problems with your approach are:
You have to remember to do this error handling. If you forget, you'll get a silent failure.
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.