expressjs / expressjs/express

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

Open
#2,896 23 comments 70 reactions 0 assignees View on GitHub
discuss enhancement ideas
Dominant language
JavaScript
Stars
69.5k
Forks
25k
Avg merge
4d 20h
Merged PRs (30d)
9

Description

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

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

Instead of:

``` js
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](http://eslint.org/docs/2.0.0/rules/no-unused-vars) 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.

``` js
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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.