make application routing callback behave the same as middleware #1358

wants to merge 2 commits into

2 participants


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 {
      } else if (arity < 4) {
        layer.handle(req, res, next);
      } else {

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) {
        } else if (err && fn) {
          if (fn.length < 4) return callbacks(err);
          fn(err, req, res, callbacks);
        } else if (fn) {
          fn(req, res, callbacks);
        } else {
      } catch (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.

expressjs member
tj commented Oct 9, 2012

great, I over-looked this, thanks! do you mind adding a test? I can do so if you dont have time


OK. I'll try to add a test. I'm not familiar with the convention of test code. Anyway, I'll give it a try:)


By adding test code, I found the fix I made yesterday still has a defect: if no error is propagated, an error-handling routing callback should be skipped, but we should skip only this one rather than calling nextRoute() to skip all remaining callbacks. So I make a further fix along with the test code.

expressjs member
tj commented Oct 10, 2012


@tj tj closed this Oct 10, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment