Express: catch error of "toplevel" app.use-middlewear in router error handler

[skleeschulte]

In the docs (http://expressjs.com/guide/error-handling.html) I read "Define error-handling middleware like other middleware, except with four arguments instead of three, specifically with the signature (err, req, res, next))" and "next(err) will skip all remaining handlers in the chain except for those that are set up to handle errors as described in the next section".

However, express doesn't behave as I expect from what I read in the docs. This...

var express = require('express');
var app = express();

// generate error on every request
app.use(function(req, res, next) {
  next(new Error('Always, I want to be with you'));
});

// route /test with own error handler
var testRouter = express.Router();
testRouter.use(function(err, req, res, next) {
  res.send('testRouter error handler: ' + err.message); // <-- never called, even for requests to /test
});
app.use('/test', testRouter);

// default error handler
app.use(function(err, req, res, next) {
  res.send('default error handler: ' + err.message);
});

module.exports = app;

...will always return default error handler: Always, I want to be with you, regardless of the requested resource. If next(err) would really "skip all remaining handlers in the chain except for those that are set up to handle errors", the result for requests to /test should be testRouter error handler: Always, I want to be with you.

I stumbled upon this when trying to catch errors from a "globally" applied bodyParser in a router which handles API requests (because I need to have a different error output there).

[dougwilson]

Hi! What you describe is expected behavior. You can find out more in an old issue: https://github.com/strongloop/express/issues/2633

The gist is that app.use('/test', testRouter) is technically app.use('/test', function (req, res, next) {}), since all testRouter is is a function (you can see for yourself by doing testRouter.toString(). Because of this, the top router does not distinguish sub routers from any other middleware, and of course without a way to pass the err to it, there's not way to invoke it with the error.

[skleeschulte]

Well, that's bad news. This means I cannot put all the logic for a specific route in one external module, because I need to add the error handler for that route and all the requirements it has to the top-level. Or I have to initialize all the middleware in external route modules, although this will cause redundant code. Why can't the signature of a router be changed to function (err, req, res, next)? The router can still pass on the error if there is no appropriate error handler.
In addition I think the documentation is not clear. "next(err) will skip all remaining handlers in the chain except for those that are set up to handle errors" - to me it's not clear that error handlers in route modules are not in that chain.

[skleeschulte]

Ok, I found a solution to encapsulate router and error handler in a module - I export both, router and error handler from the module and add both as middleware. Thus in app.js I can do:

var api = require('./routes/api/v1.0/api');
app.use('/api/v1.0', api.router);
app.use('/api/v1.0', api.errorHandler);

I guess the router signature cannot simply be changed to function (err, req, res, next) because then the router wouldn't be able to handle non-error requests anymore? And I further guess this cannot easily be changed because routers are in some way nested promises?

But still, the documentation should be more clear on this...

[dougwilson]

Right. Any clarity you can help you provide to the documentation is very welcome! Just open up an issue or PR over at https://github.com/strongloop/expressjs.com :) It's not always easy for us to come up with docs from the eyes of outside the knowledge circle, so we always welcome documentation feedback from our users :)

As for your pattern above,

var api = require('./routes/api/v1.0/api');
app.use('/api/v1.0', api.router);
app.use('/api/v1.0', api.errorHandler);

If you are doing this, then why can't you simple add the error handlers in the api.router? Errors that occur within api.router will still invoke it's own error handlers. Can you give a little example of what you would have inside your ./routes/api/v1.0/api file and and I see what it looks like and perhaps provide a suggestion?

[skleeschulte]

Thank you for your help! If I put the error handlers into the api.router, they don't catch errors that occur in other middleware outside of that router (that's what we're talking about, isn't it? :) ). Requests to /api/v1.0 shall be processed by "application level" middleware first (e.g. bodyParser), which might produce an error. For example, bodyParser will produce an error on invalid json data in the request body. In this case I need to generate an error response in the format required by the api. That's why I want the api error handler to catch all the errors, not only errors from within the api router.

./routes/api/v1.0/api.js looked something like the following:

var express = require('express');
var router = express.Router();

router.use('/endpoint', require('./endpoint'));

router.use(function (err, req, res, next) {
  // all errors need to be returned in json
  res.status(err.status || 500);
  res.json({ error: err.message });
});

module.exports = router;

And now I changed it to something like this:

var express = require('express');
var router = express.Router();

router.use('/endpoint', require('./endpoint'));

var errorHandler = function (err, req, res, next) {
  // all errors need to be returned in json
  res.status(err.status || 500);
  res.json({ error: err.message });
};

module.exports.router = router;
module.exports.errorHandler = errorHandler;

[dougwilson]

they don't catch errors that occur in other middleware outside of that router (that's what we're talking about, isn't it? :) ). Requests to /api/v1.0 shall be processed by "application level" middleware first (e.g. bodyParser), which might produce an error

Ah, ha! Sorry, I totally forgot that from your initial post :) Yes, indeed, those errors would not climb into your sub router, by bad :)

To me, the way it works is basically just like try-catch:

try {
  // main router
  parseBody()

  try {
    // sub router
    doThings()
  } catch (e) {
    // error handlers in your sub router--they wouldn't catch parseBody(), just like JS itself
  }
} catch (e) {
  // error handlers on your main router
  // of course this catches all fall-through errors, just like JS itself
}

Another thing you can try is, if possible, to move body parsing closer to where you're going to actually use it, such that you are not parsing bodies to 404 routes or unauthenticated resources. https://github.com/expressjs/body-parser#express-route-specific provides a basic example of this pattern.