diff --git a/lib/server-app.js b/lib/server-app.js index 13f993a8..02e8972f 100644 --- a/lib/server-app.js +++ b/lib/server-app.js @@ -161,7 +161,15 @@ proto.middleware = function(name, paths, handler) { this._skipLayerSorting = true; this.use(paths, handler); - this._router.stack[this._router.stack.length - 1].phase = fullPhaseName; + + var layer = this._findLayerByHandler(handler); + if (layer) { + // Set the phase name for sorting + layer.phase = fullPhaseName; + } else { + debug('No matching layer is found for %s %s', fullPhaseName, handlerName); + } + this._skipLayerSorting = false; this._sortLayersByPhase(); @@ -169,6 +177,35 @@ proto.middleware = function(name, paths, handler) { return this; }; +/* + * Find the corresponding express layer by handler + * + * This is needed because monitoring agents such as NewRelic can add handlers + * to the stack. For example, NewRelic adds sentinel handler. We need to search + * the stackto find the correct layer. + */ +proto._findLayerByHandler = function(handler) { + // Other handlers can be added to the stack, for example, + // NewRelic adds sentinel handler. We need to search the stack + for (var k = this._router.stack.length - 1; k >= 0; k--) { + if (this._router.stack[k].handle === handler || + // NewRelic replaces the handle and keeps it as __NR_original + this._router.stack[k].handle['__NR_original'] === handler + ) { + return this._router.stack[k]; + } else { + // Aggressively check if the original handler has been wrapped + // into a new function with a property pointing to the original handler + for (var p in this._router.stack[k].handle) { + if (this._router.stack[k].handle[p] === handler) { + return this._router.stack[k]; + } + } + } + } + return null; +}; + // Install our custom PhaseList-based handler into the app proto.lazyrouter = function() { var self = this; diff --git a/test/app.test.js b/test/app.test.js index 18f326c4..82a4456a 100644 --- a/test/app.test.js +++ b/test/app.test.js @@ -65,6 +65,62 @@ describe('app', function() { }); }); + it('allows extra handlers on express stack during app.use', function(done) { + function handlerThatAddsHandler(name) { + app.use(namedHandler('extra-handler')); + return namedHandler(name); + } + + var myHandler; + app.middleware('routes:before', + myHandler = handlerThatAddsHandler('my-handler')); + var found = app._findLayerByHandler(myHandler); + expect(found).to.be.object; + expect(myHandler).to.equal(found.handle); + expect(found).have.property('phase', 'routes:before'); + executeMiddlewareHandlers(app, function(err) { + if (err) return done(err); + expect(steps).to.eql(['my-handler', 'extra-handler']); + done(); + }); + }); + + it('allows handlers to be wrapped as __NR_handler on express stack', + function(done) { + var myHandler = namedHandler('my-handler'); + var wrappedHandler = function(req, res, next) { + myHandler(req, res, next); + }; + wrappedHandler['__NR_handler'] = myHandler; + app.middleware('routes:before', wrappedHandler); + var found = app._findLayerByHandler(myHandler); + expect(found).to.be.object; + expect(found).have.property('phase', 'routes:before'); + executeMiddlewareHandlers(app, function(err) { + if (err) return done(err); + expect(steps).to.eql(['my-handler']); + done(); + }); + }); + + it('allows handlers to be wrapped as a property on express stack', + function(done) { + var myHandler = namedHandler('my-handler'); + var wrappedHandler = function(req, res, next) { + myHandler(req, res, next); + }; + wrappedHandler['__handler'] = myHandler; + app.middleware('routes:before', wrappedHandler); + var found = app._findLayerByHandler(myHandler); + expect(found).to.be.object; + expect(found).have.property('phase', 'routes:before'); + executeMiddlewareHandlers(app, function(err) { + if (err) return done(err); + expect(steps).to.eql(['my-handler']); + done(); + }); + }); + it('injects error from previous phases into the router', function(done) { var expectedError = new Error('expected error');