Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use named functions for error handlers #50

Open
CanisLupusXenonis opened this issue Oct 28, 2016 · 2 comments
Open

Use named functions for error handlers #50

CanisLupusXenonis opened this issue Oct 28, 2016 · 2 comments

Comments

@CanisLupusXenonis
Copy link

Inspired by expressjs/express#2896, but proposed here because as a breaking change it likely won't make it into Express 4.0 anyway, and Express 5.0 references this.

Instead of using the number of arguments to differentiate between error and normal handlers, is there any reason not to use named functions? Example:

route.get(function(req, res, next) {
    next('oops')
}, function(err, req, res, next) {
    res.end('Error')
})

Instead, we could say:

route.get(function(req, res, next) {
    next('oops')
}, function error(err, req, res) {
    res.end('Error')
})

And then just check for this.name instead of fn.length in layer.js:63.
Should work in all places where error handlers can be specified here and in express unless I'm overlooking something, and would remove the need (and warnings) for unused parameters.
Could even still allow the old style (but make it deprecated?) to not to break existing code.

Thoughts? :)

@blakeembrey
Copy link
Member

Personally, named functions are even more error prone than checking arrity. There's no advantage to the change. Also, it's pretty common to have an error handler that still passes on to next so we'd still need to handle three arguments. If there were to be a change to error handling, I'd suggest it uses less magic and not more.

@jmar777
Copy link

jmar777 commented Oct 28, 2016

It's probably also worth noting that this proposal would be incompatible with inline arrow functions. Right now the only way to get the name property set is through a previous assignment statement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants