express: Define error-handling middleware functions explicitly (without arity detection)

I propose adding an explicit app.error method for defining error-handling middleware functions:

app.error(function(err, req, res, next) {
  res.status(500).send('Something broke!');
});

Instead of:

app.use(function(err, req, res, next) {
  res.status(500).send('Something broke!');
});

It’s so easy to forget the next argument when it’s not being used in the body of the function, and that changes the whole meaning of the middleware. express is one of the only packages that has this pattern. Others that used this pattern in the past (i.e. superagent) have since removed it.

Furthermore, this clashes with popular linting rules like ESLint’s no-unused-vars rule which enforce that all named arguments must be used in the function body. Users who see this rule and remove the un-unsed next parameter will be unwittingly changing the behavior of their program.

No one expects removing an unused parameter to change the behavior of a program.

app.use(function(err, req, res, next /* <-- unused, guess I'll remove this... */) {
  res.status(500).send('Something broke!');
});

The four argument middleware convention should be deprecated in favor of app.error, but support for it could remain for a long time, or even indefinitely. I just want to be able to recommend that folks use app.error going forward.

About this issue

Original URL
State: open
Created 8 years ago
Reactions: 68
Comments: 23 (15 by maintainers)

Most upvoted comments

Regardless to various things to work out, what I hear loud and clear: Express should not use argument arity to differentiate between error and non-error handlers, and I 100% agree.

+19

dougwilson on Feb 29, 2016

Major 👍 to this - my linter complains about the unused parameter all the time even though I can’t remove it.

billinghamj on Feb 20, 2016

The interface looks lovely. I am assuming the extended example looks something like this:

app.error(function(err, req, res, next) {
  if (err.code === 404) res.status(err.code).render('404');
  else res.status(err.code).send(err.string);
});

hacksparrow on Feb 19, 2016

If anyone is interested, I opened a PR in the router package for this idea. Would appreciate any feedback. https://github.com/pillarjs/router/pull/59

wesleytodd on Oct 4, 2017

Nice option, There is another option,

/* error parameter will be at end. To maintain standards*/
app.use(function(req, res, next, err) { 
  res.status(500).send('Something broke!');
});

Another one option. Using chaining to handle error. This will give power to handle error at all levels.

app.get('/',function(req, res, next) { 
  res.status(500).send('Something broke!');
}).error(function(err){
    /* Error Handler*/
});

dlinx on Feb 28, 2016

This, this this! I have thought this for a very long time. The implicit behavior of the error handlers is confusing to beginners and advanced users alike. I would support making this to 5.0 and explicitly deprecating the old way, for future removal. And possibly even adding as a new feature in 4.0 as a point release. Thanks @feross for bringing this up.

wesleytodd on Feb 19, 2016