visionmedia/express — pull request #1358 by seansoong (2012-10-08 07:31:37+00:00)
Title: make application routing callback behave the same as middleware

Hi, there. I have a question about implementing an application routing callback with the first parameter being err.

The API Reference says under the section of app.VERB(path, [callback...], callback)：

| Multiple callbacks may be give, all are treated equally, and behave just like middleware,
| with the one exception that these callbacks may invoke next('route') to bypass the remaining route callback(s).

But when it comes to error handling, application routing callback behaves a little differently than middleware.

In the case of middleware, if it's declared with the first parameter being err, it won't be called (will be skipped) if there's no error generated above this middleware. I can see that from the source code of connect (lib/proto.js):

var arity = layer.handle.length;
if (err) {
if (arity === 4) {
layer.handle(err, req, res, next);
} else {
next(err);
}
} else if (arity < 4) {
layer.handle(req, res, next);
} else {
next();
}

But in the case of application routing callback, the source code of express (lib/router/index.js) says:

function callbacks(err) {
var fn = route.callbacks[i++];
try {
if ('route' == err) {
nextRoute();
} else if (err && fn) {
if (fn.length < 4) return callbacks(err);
fn(err, req, res, callbacks);
} else if (fn) {
fn(req, res, callbacks);
} else {
nextRoute(err);
}
} catch (err) {
callbacks(err);
}
}

if callbacks() is called without an err, the code path goes to this branch:

else if (fn) {
fn(req, res, callbacks);
}

Whether fn is implemented with the first parameter being err or not, it always gets called with (req, res, callbacks).

Suppose we implement fn with an err parameter, i.e. function (err, req, res, next) { ... } , then we should be careful, if there are less than 4 arguments passed we have to shift the arguments one position to the right. This is really a burden for application developers.

Couldn't it be better if we make application routing callback behave the same as middleware? We can change the code to:

else if (fn && fn.length < 4) {
fn(req, res, callbacks);
}

That's what this commit is about.