Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catch errors in promise if promise returned and forward them to error handling middleware #3031

Closed
meghprkh opened this issue Jun 28, 2016 · 1 comment

Comments

@meghprkh
Copy link

Currently, Express forwards errors thrown in a route using throw and forwards them to error handling middleware. But it doesnot do something similar if a promise is returned. I have to manually append .catch(next) to the promise to get it forwarded to error handling middleware. For example the following is my code:

// errorHandlers.js
function sequelizeValidationError (err, req, res, next) {
  if (err.name && err.name == 'SequelizeValidationError')
    res.status(400).send(err.errors)
  else next(err)
}

// auth.js
router.post ('/register',  middleware.isNotAuthenticated, (req, res, next) => {
  const { email, password, name } = req.body;

  return models.User.find({where : { email }}).then(user => {
    if (user) {
      if (user.password == password) sendToken(user.id, res);
      else res.sendStatus(401);
    } else {
      return models.User.create({
        email, password, name
      }).then(user => {
        sendToken(user.id, res);
      })
    }
  }).catch(next)
})

// index.js
router.use('/auth', require('./auth'))

router.use(errorHandlers.sequelizeValidationError)

It currently fails if I had to forgot to write the catch, instead of automatically calling the error handling middleware. This can result to errors if I somewhere fail to write catch andd IMHO it is inconsistent too.

Also a small question, can I make a middleware to handle this case temporarily? (sorry I know I shouldnt ask this but please)

@blakeembrey
Copy link
Member

This is a duplicate of #3029, #3026 and #2259. I would follow #2259 for more information. To fix this today, you can wrap it in something like https://github.com/blakeembrey/async-middleware.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants