From 704b06b49cbcfb3dfbc3a4d9249314caa9f30f67 Mon Sep 17 00:00:00 2001 From: calebmer Date: Wed, 23 Sep 2015 13:48:19 -0400 Subject: [PATCH 1/4] Add middleware promise support --- lib/router/layer.js | 51 ++++++++++++------ lib/router/route.js | 3 ++ package.json | 13 ++--- test/middleware.promise.js | 108 +++++++++++++++++++++++++++++++++++++ 4 files changed, 153 insertions(+), 22 deletions(-) create mode 100644 test/middleware.promise.js diff --git a/lib/router/layer.js b/lib/router/layer.js index fe9210cb9d..fb1450ae57 100644 --- a/lib/router/layer.js +++ b/lib/router/layer.js @@ -49,6 +49,37 @@ function Layer(path, options, fn) { } } +/** + * Ubiquitous function that handles any type + * + * Takes any number of parameters, but the last must always be a `next` function. + * + * @api private + */ + +Layer.prototype.handle_any = function (/* ...args, next */) { + var self = this; + var args = Array.prototype.slice.call(arguments); + var next = args.pop(); + var hasNextBeenCalled = function () { return next._layer === self; }; + + var handle = this.handle; + var result; + + try { + result = handle.apply(undefined, args.concat([next])); + } catch (err) { + return next(err); + } + + // If the result is a promise + if (result != null && typeof result.then === 'function') { + var onFulfilled = function () { if (!hasNextBeenCalled()) { next(); } }; + var onRejected = function (error) { if (!hasNextBeenCalled()) { next(error); } }; + result.then(onFulfilled, onRejected); + } +}; + /** * Handle the error for the layer. * @@ -60,18 +91,12 @@ function Layer(path, options, fn) { */ Layer.prototype.handle_error = function handle_error(error, req, res, next) { - var fn = this.handle; - - if (fn.length !== 4) { + if (this.handle.length !== 4) { // not a standard error handler return next(error); } - try { - fn(error, req, res, next); - } catch (err) { - next(err); - } + this.handle_any(error, req, res, next); }; /** @@ -84,18 +109,12 @@ Layer.prototype.handle_error = function handle_error(error, req, res, next) { */ Layer.prototype.handle_request = function handle(req, res, next) { - var fn = this.handle; - - if (fn.length > 3) { + if (this.handle.length > 3) { // not a standard request handler return next(); } - try { - fn(req, res, next); - } catch (err) { - next(err); - } + this.handle_any(req, res, next); }; /** diff --git a/lib/router/route.js b/lib/router/route.js index 2788d7b735..68e5e12936 100644 --- a/lib/router/route.js +++ b/lib/router/route.js @@ -121,6 +121,9 @@ Route.prototype.dispatch = function dispatch(req, res, done) { return done(err); } + // Set a reference to the current layer + next._layer = layer; + if (layer.method && layer.method !== method) { return next(err); } diff --git a/package.json b/package.json index 3f299bada8..88c482515a 100644 --- a/package.json +++ b/package.json @@ -55,21 +55,22 @@ }, "devDependencies": { "after": "0.8.1", - "ejs": "2.3.3", - "istanbul": "0.3.17", - "marked": "0.3.5", - "mocha": "2.2.5", - "should": "7.0.2", - "supertest": "1.0.1", + "bluebird": "^2.10.1", "body-parser": "~1.13.3", "connect-redis": "~2.4.1", "cookie-parser": "~1.3.5", "cookie-session": "~1.2.0", + "ejs": "2.3.3", "express-session": "~1.11.3", + "istanbul": "0.3.17", "jade": "~1.11.0", + "marked": "0.3.5", "method-override": "~2.3.5", + "mocha": "2.2.5", "morgan": "~1.6.1", "multiparty": "~4.1.2", + "should": "7.0.2", + "supertest": "1.0.1", "vhost": "~3.0.1" }, "engines": { diff --git a/test/middleware.promise.js b/test/middleware.promise.js new file mode 100644 index 0000000000..d3de632a74 --- /dev/null +++ b/test/middleware.promise.js @@ -0,0 +1,108 @@ + +var express = require('../'); +var request = require('supertest'); +var Promise = require('bluebird'); + +describe('middleware', function () { + describe('promises', function () { + it('resolving will be waited on', function (done) { + var app = express(); + var count = 0; + + app.use(function (req, res) { + count++; + return new Promise(function (resolve, reject) { + count++; + setTimeout(function () { + count++; + resolve(); + }, 5); + }); + }); + + app.use(function (req, res) { + count.should.be.exactly(3); + res.end('Awesome!') + }); + + request(app) + .get('/') + .expect(200, done); + }); + + it('rejecting will trigger error handlers', function (done) { + var app = express(); + + app.use(function (req, res) { + return new Promise(function (resolve, reject) { + reject(new Error('Happy error')); + }); + }); + + request(app) + .get('/') + .expect(500, done); + }); + + it('will be ignored if next is called', function (done) { + var app = express(); + var count = 0; + + app.use(function (req, res, next) { + count++; + return new Promise(function (resolve, reject) { + count++; + next(); + setTimeout(function () { + count++; + resolve(); + }, 5); + }); + }); + + app.use(function (req, res) { + count.should.be.exactly(2); + res.end('Awesome!'); + }); + + request(app) + .get('/') + .expect(200, done); + }); + + it('can be used in error handlers', function (done) { + var app = express(); + var count = 0; + + app.use(function (req, res, next) { + count++; + next(new Error('Happy error')); + }); + + app.use(function (error, req, res, next) { + count++; + return new Promise(function (resolve, reject) { + count++ + setTimeout(function () { + count++; + next(error); + resolve(); + }, 5); + }); + }); + + app.use(function () { + done(new Error('This should never be reached')); + }); + + app.use(function (error, req, res, next) { + count.should.be.exactly(4); + res.end('Awesome!'); + }); + + request(app) + .get('/') + .expect(200, done); + }); + }); +}); From 36565bb087c05c991f1ee93a69a088a42c327f7e Mon Sep 17 00:00:00 2001 From: calebmer Date: Wed, 23 Sep 2015 13:54:35 -0400 Subject: [PATCH 2/4] Remove auto-next call --- lib/router/layer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/router/layer.js b/lib/router/layer.js index fb1450ae57..120fdddbf2 100644 --- a/lib/router/layer.js +++ b/lib/router/layer.js @@ -74,7 +74,7 @@ Layer.prototype.handle_any = function (/* ...args, next */) { // If the result is a promise if (result != null && typeof result.then === 'function') { - var onFulfilled = function () { if (!hasNextBeenCalled()) { next(); } }; + var onFulfilled = function () { /* Silence... */ }; var onRejected = function (error) { if (!hasNextBeenCalled()) { next(error); } }; result.then(onFulfilled, onRejected); } From 2674ad80b2db932fa03e39f8a53ac9f3c0885522 Mon Sep 17 00:00:00 2001 From: calebmer Date: Wed, 23 Sep 2015 14:01:43 -0400 Subject: [PATCH 3/4] Rename property --- lib/router/layer.js | 2 +- lib/router/route.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/router/layer.js b/lib/router/layer.js index 120fdddbf2..05d6861a6b 100644 --- a/lib/router/layer.js +++ b/lib/router/layer.js @@ -61,7 +61,7 @@ Layer.prototype.handle_any = function (/* ...args, next */) { var self = this; var args = Array.prototype.slice.call(arguments); var next = args.pop(); - var hasNextBeenCalled = function () { return next._layer === self; }; + var hasNextBeenCalled = function () { return next._currentLayer === self; }; var handle = this.handle; var result; diff --git a/lib/router/route.js b/lib/router/route.js index 68e5e12936..93f27d4af6 100644 --- a/lib/router/route.js +++ b/lib/router/route.js @@ -122,7 +122,7 @@ Route.prototype.dispatch = function dispatch(req, res, done) { } // Set a reference to the current layer - next._layer = layer; + next._currentLayer = layer; if (layer.method && layer.method !== method) { return next(err); From 8a6415f93c54b8ff37f91e0148adb28bdb96f79f Mon Sep 17 00:00:00 2001 From: calebmer Date: Wed, 23 Sep 2015 14:03:53 -0400 Subject: [PATCH 4/4] Remove test for unused feature --- test/middleware.promise.js | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/test/middleware.promise.js b/test/middleware.promise.js index d3de632a74..5708204d24 100644 --- a/test/middleware.promise.js +++ b/test/middleware.promise.js @@ -5,31 +5,6 @@ var Promise = require('bluebird'); describe('middleware', function () { describe('promises', function () { - it('resolving will be waited on', function (done) { - var app = express(); - var count = 0; - - app.use(function (req, res) { - count++; - return new Promise(function (resolve, reject) { - count++; - setTimeout(function () { - count++; - resolve(); - }, 5); - }); - }); - - app.use(function (req, res) { - count.should.be.exactly(3); - res.end('Awesome!') - }); - - request(app) - .get('/') - .expect(200, done); - }); - it('rejecting will trigger error handlers', function (done) { var app = express();