Generic route handler is called after a response was provided by a specific route handler

Question

I'm trying to add authentication to my Express-based server. I am noticing some strange behavior of the routing.

I've distilled the problem to this Express code:

app.get('/', function (req, res) {
    console.log('this is reached first');
    res.send('Hello');
});

app.get('/', function (req, res) {
    console.log('this is not reached');
});

app.get('*', function (req, res) {
    console.log('this is reached');
});

Upon requesting '/' the 1st handler is called. It provides a response and does not call next(). Therefore I am surprised to see that the 3rd handler ('*') is also called! Another surprise is that the response ('res') passed to the 3rd handler is not the same as the one passed to the first handler. (If I were to call next() from the 1st handler then the 2nd handler would be called, and with the same response object.)

Now for my real scenario: I want to process requests and verify authentication in a global manner. Some routes, however, should remain available for non-authenticated users. I based my solution on Zikes' answer. I routed the "free" paths first. Then I included an route handler for all ('*'). It would call next() if the user is authenticated or next(err) otherwise. Following are all the "restricted" routes. And finally, my own error handler using app.use. It looks something like this:

app.use(app.router);
app.use(function(err, req, res, next) {
    console.log('An error occurred: ' + err);
    res.send(401, 'Unauthorized');
});

app.get('/', function (req, res) {
    res.send('Hello all');
});

app.all('*', function (req, res, next) {
    if (req.user) {
        next(); // authorized
    } else {
        next(new Error('401')); // unauthorized
    }
});

app.get('/members', function (req, res) {
    res.send('Hello member');
});

This works rather well, blocking access to '/members'. However, it has bug: the authentication check and error handling happens even when accessing the non-restricted path ('/'). After the intended response is sent, the error handler tries to send a 401 error response. The latter does not get sent, but the code should not run.

Also, a side effect of this mechanism is that unauthenticated users get an 401 error for non-existing pages. I may want to return a 404 in some of those cases. But now I'm just pushing it...

I kinda have 2 questions:

1. Is this behavior a bug? Should the general handler be called without next being called?
2. What would be a good solution for hooking many but not all routes, without having to mark them individually?

Answer 1

For the first question, my guess is that the browser is sending more than one request.

For example, when you browse to http://localhost:3000/ Chrome will also request the http://localhost:3000/favicon.ico

You can print the request that is being caught with something like this:

app.get('*', function (req, res) {
    console.log('this is reached');
    console.log('url ' + req.url );
});

Answer 2

app.get('/', function(req, res, next) {
    console.log('this is reached first.');
    next();
}, function (req, res) {
    console.log('this is reached last.');
    res.send('Hello all');
});

I would typically construct it like this:

var auth = require('../modules/authenticate');

app.get('/', auth.requireLogin, routes.index.get);
app.delete('/items/:id', auth.requireLogin, auth.permission.item.delete, routes.item.delete);

Answer 3

First, you probably have two requests when hitting home (favicon.ico). use app.use(express.logger('dev')) to log requests. Also, you can try blocking that path:

app.use('/favicon.ico', function(req, res) {
  console.log('FAVICON')
  res.send(404)
}

Second, you want to handle your errors. A good way to send errors:

var err = new Error('this is my error message.')
err.status = 404 // You want to send a "Not Found" page
next(err)

app.use(function(err, req, res, next) {
  var status = err.status
  if (status === 404) res.send(404, 'Page not found.');
  else res.send(500, 'Something went wrong.');
})

All handled errors should have a status.