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:
- Handle your errors immediately, where they’re generated.
next(err)
really meansres.send(err)
. It’s not an alternative tothrow err
. If you need to log and shut down, pass it off to an error handling module instead of stuffing it innext(err)
where it will be swallowed. 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 fornext(err)
calls.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 inuncaughtException
, 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.- 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)
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:
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: