From 7eba938a22a1b2dc1b09e5a3b76a23bd0f66f7e1 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Thu, 17 Sep 2020 12:05:11 -0700 Subject: [PATCH 1/3] Use diagnostics_channel in layers --- lib/layer.js | 40 ++++++++++++++ test/diagnostics-channel.js | 104 ++++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+) create mode 100644 test/diagnostics-channel.js diff --git a/lib/layer.js b/lib/layer.js index 60a737f5..ca8abbb5 100644 --- a/lib/layer.js +++ b/lib/layer.js @@ -15,6 +15,18 @@ var pathRegexp = require('path-to-regexp') var debug = require('debug')('router:layer') +/** + * Diagnostic channels + * @private + */ +var onHandleRequest +var onHandleError +try { + var dc = require('diagnostics_channel') + onHandleRequest = dc.channel('express.layer.handle_request') + onHandleError = dc.channel('express.layer.handle_error') +} catch (err) { } + /** * Module variables. * @private @@ -41,6 +53,7 @@ function Layer(path, options, fn) { this.params = undefined this.path = undefined this.regexp = pathRegexp(path, this.keys = [], opts) + this.routingPath = path // set fast path flags this.regexp.fast_star = path === '*' @@ -65,9 +78,23 @@ Layer.prototype.handle_error = function handle_error(error, req, res, next) { return next(error) } + req.layerStack = req.layerStack || [] + req.layerStack.push(this) + + if (onHandleError && onHandleError.shouldPublish()) { + onHandleError.publish({ + error: error, + request: req, + response: res, + layer: this + }) + } + try { fn(error, req, res, next) + req.layerStack.pop() } catch (err) { + req.layerStack.pop() next(err) } } @@ -89,9 +116,22 @@ Layer.prototype.handle_request = function handle(req, res, next) { return next() } + req.layerStack = req.layerStack || [] + req.layerStack.push(this) + + if (onHandleRequest && onHandleRequest.shouldPublish()) { + onHandleRequest.publish({ + request: req, + response: res, + layer: this + }) + } + try { fn(req, res, next) + req.layerStack.pop() } catch (err) { + req.layerStack.pop() next(err) } } diff --git a/test/diagnostics-channel.js b/test/diagnostics-channel.js new file mode 100644 index 00000000..b457d041 --- /dev/null +++ b/test/diagnostics-channel.js @@ -0,0 +1,104 @@ + +var Router = require('../') + , assert = require('assert'); + +var dc = require('diagnostics_channel'); +var onHandleRequest = dc.channel('express.layer.handle_request'); +var onHandleError = dc.channel('express.layer.handle_error'); + +function mapProp(prop) { + return function mapped(obj) { + return obj[prop]; + }; +} + +function mapAndJoin(prop) { + return function (list) { + return list.map(mapProp(prop)).join(''); + } +} + +function noop() { } + +describe('diagnostics_channel', function () { + var joinLayerStack = mapAndJoin('routingPath'); + var handleRequest; + var handleError; + + onHandleRequest.subscribe(function (message) { + handleRequest = message; + }); + + onHandleError.subscribe(function (message) { + handleError = message; + }); + + it('use has no layers with a path', function (done) { + var router = new Router(); + + router.use(function (req, res) { + res.end(); + }); + + function end() { + assert.strictEqual(joinLayerStack(handleRequest.request.layerStack), '/'); + done(); + } + + router.handle({ url: '/', method: 'GET' }, { end }, noop); + }); + + it('regular routes have a layer with a path', function (done) { + var router = new Router(); + + router.get('/hello/:name', function (req, res) { + res.end(); + }); + + function end() { + assert.strictEqual(joinLayerStack(handleRequest.request.layerStack), '/hello/:name/'); + done(); + } + + router.handle({ url: '/hello/world', method: 'GET' }, { end }, noop); + }); + + it('nested routes have multiple layers with paths', function (done) { + var outer = new Router(); + var inner = new Router(); + + inner.get('/:name', function (req, res) { + res.end(); + }); + + outer.use('/hello', inner); + + function end() { + assert.strictEqual(joinLayerStack(handleRequest.request.layerStack), '/hello/:name/'); + done(); + } + + outer.handle({ url: '/hello/world', method: 'GET' }, { end }, noop); + }); + + it('errors send through a different channel', function (done) { + var router = new Router(); + var error = new Error('fail'); + + router.get('/hello/:name', function (req, res) { + throw error; + }); + + router.use(function (err, req, res, next) { + res.end(); + }); + + function end() { + assert.strictEqual(joinLayerStack(handleRequest.request.layerStack), '/hello/:name/'); + assert.strictEqual(handleError.error, error); + done(); + } + + router.handle({ url: '/hello/world', method: 'GET' }, { end }, noop); + }); +}); From 231f3eecf7685fbdb9bd7d916866c0b155ad19c8 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Fri, 18 Sep 2020 15:09:42 -0700 Subject: [PATCH 2/3] Fix channel names --- lib/layer.js | 4 ++-- test/diagnostics-channel.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/layer.js b/lib/layer.js index ca8abbb5..b68e124f 100644 --- a/lib/layer.js +++ b/lib/layer.js @@ -23,8 +23,8 @@ var onHandleRequest var onHandleError try { var dc = require('diagnostics_channel') - onHandleRequest = dc.channel('express.layer.handle_request') - onHandleError = dc.channel('express.layer.handle_error') + onHandleRequest = dc.channel('router.layer.handle_request') + onHandleError = dc.channel('router.layer.handle_error') } catch (err) { } /** diff --git a/test/diagnostics-channel.js b/test/diagnostics-channel.js index b457d041..13b08b2b 100644 --- a/test/diagnostics-channel.js +++ b/test/diagnostics-channel.js @@ -3,8 +3,8 @@ var Router = require('../') , assert = require('assert'); var dc = require('diagnostics_channel'); -var onHandleRequest = dc.channel('express.layer.handle_request'); -var onHandleError = dc.channel('express.layer.handle_error'); +var onHandleRequest = dc.channel('router.layer.handle_request'); +var onHandleError = dc.channel('router.layer.handle_error'); function mapProp(prop) { return function mapped(obj) { From bae6c122951e7ab3c933150457244e46d5537522 Mon Sep 17 00:00:00 2001 From: Stephen Belanger Date: Tue, 22 Sep 2020 16:14:07 -0700 Subject: [PATCH 3/3] Fix layerStack timing issue --- index.js | 9 +++++++++ lib/layer.js | 10 ---------- lib/route.js | 9 +++++++++ test/diagnostics-channel.js | 4 +++- 4 files changed, 21 insertions(+), 11 deletions(-) diff --git a/index.js b/index.js index 3608f26a..e3fd78a3 100644 --- a/index.js +++ b/index.js @@ -164,6 +164,7 @@ Router.prototype.handle = function handle(req, res, callback) { var self = this var slashAdded = false var paramcalled = {} + var matchedLayer = false // middleware and routes var stack = this.stack @@ -286,6 +287,14 @@ Router.prototype.handle = function handle(req, res, callback) { : layer.params var layerPath = layer.path + if (!matchedLayer) { + matchedLayer = true + req.layerStack = req.layerStack || [] + } else { + req.layerStack.pop() + } + req.layerStack.push(layer) + // this should be done for the layer self.process_params(layer, paramcalled, req, res, function (err) { if (err) { diff --git a/lib/layer.js b/lib/layer.js index b68e124f..cc64f47c 100644 --- a/lib/layer.js +++ b/lib/layer.js @@ -78,9 +78,6 @@ Layer.prototype.handle_error = function handle_error(error, req, res, next) { return next(error) } - req.layerStack = req.layerStack || [] - req.layerStack.push(this) - if (onHandleError && onHandleError.shouldPublish()) { onHandleError.publish({ error: error, @@ -92,9 +89,7 @@ Layer.prototype.handle_error = function handle_error(error, req, res, next) { try { fn(error, req, res, next) - req.layerStack.pop() } catch (err) { - req.layerStack.pop() next(err) } } @@ -116,9 +111,6 @@ Layer.prototype.handle_request = function handle(req, res, next) { return next() } - req.layerStack = req.layerStack || [] - req.layerStack.push(this) - if (onHandleRequest && onHandleRequest.shouldPublish()) { onHandleRequest.publish({ request: req, @@ -129,9 +121,7 @@ Layer.prototype.handle_request = function handle(req, res, next) { try { fn(req, res, next) - req.layerStack.pop() } catch (err) { - req.layerStack.pop() next(err) } } diff --git a/lib/route.js b/lib/route.js index 98363fe2..78cc6974 100644 --- a/lib/route.js +++ b/lib/route.js @@ -93,6 +93,7 @@ Route.prototype._methods = function _methods() { */ Route.prototype.dispatch = function dispatch(req, res, done) { + var matchedLayer = false var idx = 0 var stack = this.stack if (stack.length === 0) { @@ -138,6 +139,14 @@ Route.prototype.dispatch = function dispatch(req, res, done) { return done(err) } + if (!matchedLayer) { + matchedLayer = true + req.layerStack = req.layerStack || [] + } else { + req.layerStack.pop() + } + req.layerStack.push(layer) + if (err) { layer.handle_error(err, req, res, next) } else { diff --git a/test/diagnostics-channel.js b/test/diagnostics-channel.js index 13b08b2b..e4f38f79 100644 --- a/test/diagnostics-channel.js +++ b/test/diagnostics-channel.js @@ -71,7 +71,9 @@ describe('diagnostics_channel', function () { res.end(); }); - outer.use('/hello', inner); + outer.use('/hello', (req, res, next) => { + next(); + }, inner); function end() { assert.strictEqual(joinLayerStack(handleRequest.request.layerStack), '/hello/:name/');