node-restify: Restify doesn't provide a way to catch Promise rejections in a middleware

I want to be able to have a function that is called unhandled Promise rejections that happen inside a Restify middleware, to send the error to user. There is server.on('uncaughtException', ...) that catches all the errors, but if my middleware is async function or it returns a Promise, this handler won’t catch it.

Example:

// server creation is omitted

server.on('uncaughtException', (req, res, route, err) => {
 console.log(err);
  res.send({ success: false, error: err.message });
});

server.get('/', async (res, req, next) => {
  throw new Error('Some error');
})

If I run this example, it won’t call the error handler and will print this instead:

(node:23037) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error: Some error

Restify 4.3.0, tested with Node.js 6.10.0 (compiled with Babel) and 7.9.0 (it supports async natively)

About this issue

  • Original URL
  • State: closed
  • Created 7 years ago
  • Reactions: 1
  • Comments: 19 (5 by maintainers)

Commits related to this issue

Most upvoted comments

Hi, It’s 2019 now, and I think it’s time to discuss about Promise support on Restify.

Do we have any plans to implement Promise error-handling?

2020… any update on this?

I ran into this issue today, and created the following workaround:

function catchErrors(callback) {
  return async function errorHandler(req, res, next) {
    try {
      await callback(req, res, next)
    } catch (err) {
      if (!(err instanceof restify.errors.HttpError))
        err = new restify.errors.InternalServerError(err)
      next(err)
    }
  }
}

// wrap your async rest handlers

server.post('/posts', catchErrors(async (req, res, next) => {
  let data = req.body
  validate.newPost(data)
  await db.createObject('post', data)
  res.send(201) // CREATED
}))

// we can now modify or log our errors

server.on('InternalServer', function (req, res, err, next) {
  let cause = err.cause()
  if (cause && cause.name == 'ValidationError') {
    err.statusCode = 400
    err.body.code = 'ValidationError'
    err.body.message = cause.message
    err.body.details = cause.details
  } else {
    log.error(err)
    err.body = 'Something went wrong.'
  }
  next()
})

Hey @serge1peshcoff,

First, thank you for taking the time to open this feature request, and thank you @Doeke for taking the time to document that solution! ❤️

The restify codebase is designed to create semantically correct REST services optimized for stability and introspection. Promises are an exciting feature of ES6, but the tooling around them is still maturing. I’m not convinced that promises are at a point where we can say we wouldn’t be sacrificing the debugability of the codebase for the convenience provided by supporting that language construct as a first class citizen.

I’m definitely open to discussing this more though, and can be convinced otherwise. I’d like to hear other maintainers thoughts as well, I wouldn’t be surprised if my opinion is rather unpopular 😉. I would say that I’m strongly against introducing a compilation step such as babel for this project, it introduces a layer of indirection between the code that we maintain and what gets shipped as the final project. Even with sourcemaps, we would be sacrificing quite a bit in order to backport ES6 features to older versions of Node.

For now, if you would like to use some of the newer ES6 in your codebase, I would recommend wrapping them in a pattern that is callback friendly, @Doeke has an awesome example of this!

As an aside, I wouldn’t compare promise rejections to uncaughtException. I would strongly discourage using the uncaughtException handler as a way to recover and continue operation of a server. Node core has some pretty strong opinions about saving a process using domains and catching a top level uncaughtException; I’m inclined to agree with them. For more on this, refer to issue #1308

I’m going to close this for now. If anyone has a strong opinion about me being wrong here, we can re-open and discuss 😄

@noinkling the point of all this thing was to get the error data and send it back to the client, and for this I need my req and res objects, I can’t get them from unhandledRejection event

Hello, i’m confused about the restify team explainations. Promise are still maturing? Lol ?

Promises never swallow exceptions if you’re using async / await properly. It’s restify that’s swallowing exceptions by not sending them to the uncaughtException handler.

There is #1853, maybe #1833 addresses this.

We don’t recommend that you either swallow uncaught exceptions (something which promises do) or continue execution of your process after catching an exception. We feel strongly that this is an antipattern, and doesn’t lend itself to running robust and reliable services in production. After all, that is one of the key value propositions of restify. This guide should give you some context around how to reliably handle errors in Node.

Therefore, I highly doubt restify will ever support any promises based interface in the future – so if you’re looking for Promises support – restify is probably not your best bet.

With that said, we’re certainly not telling folks how they should be using restify or any body of software. If you choose to use Promises or continue execution after an uncaught exception, you’re free to, with all the consequences that they entail.

Also, that snippet @Doeke provided could easily become a module as a nice compromise 😄 If you release it on npm we can point people there for a solution moving forward.