Allow req.on('next', (err) => {...});

[mcasimir]

I'd like to discuss the option of emitting a 'next' event on the request object (or something similar) each time the router enters a new layer.

This would be an example of doing error logging with on('next') as opposed to relaying on error handling middlewares (and middleware order):

const router = new Router();

function globalAccessAndErrorLogging({logAccess, logError}) {
  const errorAlreadyLoggedSet = new Set();

  return (req, res, next) => {
    logAccess(req);

    req.on('close', () => {
      errorAlreadyLoggedSet.delete(req);
    });

    req.on('next', (err) => {
      if (err && !errorAlreadyLoggedSet.has(req)) {
        errorAlreadyLoggedSet.add(req);
        logError(req, err);
      }
    });

    next();
  }
}

router.use(globalAccessAndErrorLogging({
  logAccess: console.debug,
  logError: console.error
}));

The necessity to do something like this comes from an attempt to factor and provide as package any infrastructure related code that, in the company I'm currently working for, we have spread in many applications. Access and error logging is one of them.

In my experience the usage pattern we have is quite common:

mainRouter
  .use(logAccess)  // any of the usual logging middlewares (morgan / bunyan / pino ...)
  .use(apiRoutes)
  .use(logErrors); // any of the usual logging middlewares (morgan / bunyan / pino ...)

I noticed many situations in which the error logging is accidentally "shadowed" by another "final" error handler.

Is very hard to spot such situations in code reviews as the modified files does not usually contain anything wrong and any test would pass:

apiRoutes
  .use(resource1Routes);
  .use(resource2Routes);
  .use(handleApiErrors); // this swallows all the errors that would have been logged

The only solution, in the current state, is to rely on discipline: constantly re set-up the logging on each sub-router or force special patterns on all the applications, which makes such a simple problem a complicate coordination effort.

[wesleytodd]

Hey @mcasimir, I think this idea has been brought up a few times. You can see the discussion in #49 and #56. Before making more comments here, can you read through those and see if any of that resonates with you? If so, maybe you can post your thoughts there and close this?