Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Express and async/await: shall I wrap the lot with try/catch?

I have the following code:

app.post('/routes/passwordReset/:userId', async (req, res, next) => {
  var userId = req.params.userId

  let record = (await vars.connection.queryP('SELECT * FROM contacts WHERE id = ?', userId))[0]

  if (!record) {
    res.status(404).send('')
    return
  }

  // Calculate token and expiry, update database
  var token = require('crypto').randomBytes(48).toString('base64').replace(/[^a-zA-Z0-9]/g, '').substr(0, 28)
  var now = new Date()
  var inOneHour = new Date(now.getTime() + (1 * 1000 * 60 * 60))
  await vars.connection.queryP('UPDATE contacts SET recoverToken = ?, recoverTokenExpiry = ? WHERE id = ?', [ token, inOneHour, userId ])
  res.status(200).send('')
})

If I create an artificial error (e.g. the field for recoverTokenExpiry is artificially too short) I end up with:

[[12:19:55.649]] [ERROR] (node:11919) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 4): Error: ER_DATA_TOO_LONG: Data too long for column 'recoverToken' at row 1

I somehow thought that Express would have try/catch around middleware and called next(err) if an error was thrown. Maybe not so?

So, should I wrap each route with try/catch(e), doing next(e) if there is an error?

like image 801
Merc Avatar asked Apr 05 '18 04:04

Merc


2 Answers

Express does not do anything for you. It is very bare bones.

You have two options:

  1. Wrap everything in a try-catch
  2. Write your own error handling middleware as shown in the docs here.

Calling next() will pass the req and res to the next middleware in the stack.

Calling next(err), in other giving it an Exception object, will call any error middleware in the stack.


Define your error handlers in its own file:

// error-middleware.js

/**
 * Handler to catch `async` operation errors.
 * Reduces having to write `try-catch` all the time.
 */
exports.catchErrors = action => (req, res, next) => action(req, res).catch(next)

/**
 * Show useful information to client in development.
 */
exports.devErrorHandler = (err, req, res, next) => {
  err.stack = err.stack || ''
  const status = err.status || 500
  const error = { message: err.message }
  res.status(status)
  res.json({ status, error })
}

Then move your password reset logic out of app.post and into its own dedicated function:

// password-controller.js

exports.resetPassword = async (req, res) => {
  // do work
  // ...
}

This makes it easier to write unit tests for and a clear separation of concerns.

Next create your password reset route(s):

// password-reset-routes.js

const express = require('express')
const router = express.Router()

const passwordController = require('./password-controller')
const { catchErrors } = require('./error-middleware')

router.post('/routes/passwordReset/:userId', catchErrors(passwordController.resetPassword))

module.exports = router

Notice that the controller defined above is imported and used here. In addition you see that the controller action resetPassword is wrapped with catchErrors. This avoids having to write try-catch all the time and any errors will be caught by the catchErrors middleware.

Finally, wire it all up in your main app.js file:

// app.js

const express = require('express')
const { devErrorHandler } = require('./error-middleware')
const passwordResetRoutes = require('./password-reset-routes.js')
const app = express()

app.use(devErrorHandler)
app.use(passwordResetRoutes)

module.exports = app
like image 79
Francisco Mateo Avatar answered Oct 06 '22 22:10

Francisco Mateo


Well, as you're using await keyword, it means the execution flow isn't asynchronous anymore, that's why it fails and yes, you must use try-catch block every time. To avoid this, you can instead follow the asynchronous flow and use then() and catch() respectively, so you can do something like:

...
.then((status)=>{
  res.status(200).send('')
})
.catch((err)=>{
  next(err);
});
...
like image 43
Gustavo Topete Avatar answered Oct 06 '22 20:10

Gustavo Topete