node-restify: Error handling documentation is incomplete.

Edit / resolution

The documentation about error handling is incomplete and confusing. Maybe it would help to explicitly clarify the following points in the section on error handling:

  1. Handle your errors immediately, where they’re generated. next(err) really means res.send(err). It’s not an alternative to throw err. If you need to log and shut down, pass it off to an error handling module instead of stuffing it in next(err) where it will be swallowed.
  2. server.on('uncaughtException'... is how you handle any errors that were thrown in middleware / routes. It works fine for that purpose, as long as you remember the first guideline. 'uncaughtException' will not be triggered for next(err) calls.
  3. formatters are the way to customize error messages going out to the users. I don’t think they should be used for the other tasks described in this thread. They’re definitely not a viable alternative to dealing with errors in uncaughtException, because presumably, once you get there, you’re sending the user the error you want them to see… not necessarily the error that was originally generated. See point 1.
  4. For errors unrelated to middleware, remember to use process.on('uncaughtException'...

Here’s the original issue, for posterity:

Problem

Restify swallows all errors coming from route handlers. It does invoke its own error handler, but that handler never bails, regardless of whether or not an error is recoverable.

Expected

It should be up to the application developer to determine whether or not a service restart is needed. While it should be safe to swallow 4xx errors, other types of unhandled errors could leave the application in undefined state, which could lead to all sorts of trouble.

Reproduce

Restify v2.6.0 Node v0.10.20

var restify = require('restify'),
  app = restify.createServer(),
  errorHandler = require('../error-handler.js'),

  handleError = function handleError(err) {
    console.log('Caught error!'); // never happens
    setTimeout(function () {
      process.exit(1);
    }, 3000);
  },

  middlewareError =
      function middlewareError(req, res, next) {
    throw new Error('Random middleware error.');
  };


app.get('/err', function (req, res, next) {
  // This doesn't get caught.
  next( new Error('Random unrecoverable error. ' +
    'Server is now running in undefined state!') );
});

app.get('/throwerr', function (req, res, next) {
  // This doesn't get caught.
  throw new Error('Random unrecoverable error. ' +
    'Server is now running in undefined state!');
});

// This gets caught, yay!
app.use(middlewareError);

app.get('/middleware', function (req, res, next) {
  // Placeholder to invoke middlewareError.
});

// This works for middleware. Fails for routes.
app.on('uncaughtException', handleError);

// Nope. This doesn't help.
process.on('uncaughtException', handleError);

app.listen(3000, function () {
  console.log('Listening on port 3000');
});

About this issue

  • Original URL
  • State: closed
  • Created 11 years ago
  • Comments: 26 (10 by maintainers)

Most upvoted comments

Any update on this? It would be great if Restify supported connect-style error middleware, or at least had it documented somewhere that it doesn’t.

You can use the error events to handle errors on a per type basis:

function (req, res, next) {
    return next(new NotFoundError('file not found!');
}

server.on('NotFound', function(req, res, err, next) {
   // handle your error here
});

It does mean you have to be very explicit about your error types. There’s ongoing work right now to support a generic catch all handler over in #892 , allowing you to handle all error types:

server.on('restifyError', function(req, res, err next) {
    // handle all errors passed to next here, whether it's Error or NotFoundError or anything that is an instance of Error
});