foundersandcoders / open-tourism-platform

An open platform to facilitate the creation of apps to promote local tourism and business in Nazareth
MIT License
17 stars 3 forks source link

Clarify error handling #62

Closed m4v15 closed 7 years ago

m4v15 commented 7 years ago

At the moment when we catch an error on a database request we do this:

    .catch((err) => {
      const errorObj = { message: `Database Error: ${err.message}` }
      res.status(500).send(errorObj)
    })

Make and error object with a message from the err, send it back with the status code we want.

This probably isn't the best way, keeping it here for records.

We should maybe change to using boom, which would presumably be:

    .catch((err) => {
      const errorObj = { message: `Database Error: ${err.message}` }
      res.boomify(err, { message: 'Database Error'})
    })
mattlub commented 7 years ago

agree- prob use boom.

des-des commented 7 years ago

Okay, I think we should build some express middleware to handle errors

https://stackoverflow.com/questions/20021324/restful-api-node-js-mongoose-handling-errors http://expressjs.com/en/guide/error-handling.html https://www.npmjs.com/package/express-boom

const mongoErrorMiddleware = (err, req, res, next) => {
  if (!err.name === 'MongoError') return next(err)

  if (err.code === 11000) /*duplicate key*/ return res.boom.something()

  ...

  res.boom // 500 unhandled mongo error
}

const mongooseErrorMiddleware = (err, req, res, next) => {
  ...
}

You might need to look at the mongo / mongoose apis to work out how to identify these errors.

Also, try not to hard code strings like 'MongoError' as well as error codes. keep them in a file.

Thinking about this I like storing constants like

// mongo_error_codes.js
module.exports = {
  DUPLICATE_KEY: 11000,
  ...
}

there also might be npm module that does this (or poss you can get these fomr mongoose npm module)

des-des commented 7 years ago

Okay going with this approach, issue now at #71