WIP: First-class promise handling

pillarjs · Dec 5, 2015 · 1892107 · 1892107
1 parent 7ed8a4b
commit 1892107
Show file tree

Hide file tree

Showing 6 changed files with 119 additions and 17 deletions.
diff --git a/index.js b/index.js
@@ -186,7 +186,7 @@ Router.prototype.handle = function handle(req, res, callback) {
   req.baseUrl = parentUrl
   req.originalUrl = req.originalUrl || req.url
 
-  next()
+  return next()
 
   function next(err) {
     var layerError = err === 'route'
@@ -208,8 +208,11 @@ Router.prototype.handle = function handle(req, res, callback) {
 
     // no more matching layers
     if (idx >= stack.length) {
-      defer(done, layerError)
-      return
+      return new Promise(function (resolve) {
+        defer(function () {
+          resolve(done(layerError))
+        })
+      })
     }
 
     // get pathname of request
@@ -281,7 +284,7 @@ Router.prototype.handle = function handle(req, res, callback) {
     var layerPath = layer.path
 
     // this should be done for the layer
-    self.process_params(layer, paramcalled, req, res, function (err) {
+    return self.process_params(layer, paramcalled, req, res, function (err) {
       if (err) {
         return next(layerError || err)
       }
@@ -290,7 +293,7 @@ Router.prototype.handle = function handle(req, res, callback) {
         return layer.handle_request(req, res, next)
       }
 
-      trim_prefix(layer, layerError, layerPath, path)
+      return trim_prefix(layer, layerError, layerPath, path)
     })
   }
 
@@ -324,9 +327,9 @@ Router.prototype.handle = function handle(req, res, callback) {
     debug('%s %s : %s', layer.name, layerPath, req.originalUrl)
 
     if (layerError) {
-      layer.handle_error(layerError, req, res, next)
+      return layer.handle_error(layerError, req, res, next)
     } else {
-      layer.handle_request(req, res, next)
+      return layer.handle_request(req, res, next)
     }
   }
 }
@@ -425,7 +428,7 @@ Router.prototype.process_params = function process_params(layer, called, req, re
     }
   }
 
-  param()
+  return param()
 }
 
 /**

diff --git a/lib/layer.js b/lib/layer.js
@@ -15,6 +15,16 @@
 var pathRegexp = require('path-to-regexp')
 var debug = require('debug')('router:layer')
 
+/**
+ * Normalize promises for node 0.12+.
+ */
+
+function normalize (result, next) {
+  return Promise.resolve(result).then(null, function (err) {
+    return next(err || new Error('Promise was rejected with a falsy value'))
+  })
+}
+
 /**
  * Module variables.
  * @private
@@ -66,9 +76,9 @@ Layer.prototype.handle_error = function handle_error(error, req, res, next) {
   }
 
   try {
-    fn(error, req, res, next)
+    return normalize(fn(error, req, res, next), next)
   } catch (err) {
-    next(err)
+    return next(err)
   }
 }
 
@@ -90,9 +100,9 @@ Layer.prototype.handle_request = function handle(req, res, next) {
   }
 
   try {
-    fn(req, res, next)
+    return normalize(fn(req, res, next), next)
   } catch (err) {
-    next(err)
+    return next(err)
   }
 }
 

diff --git a/lib/route.js b/lib/route.js
@@ -106,7 +106,7 @@ Route.prototype.dispatch = function dispatch(req, res, done) {
 
   req.route = this
 
-  next()
+  return next()
 
   function next(err) {
     if (err && err === 'route') {
@@ -133,9 +133,9 @@ Route.prototype.dispatch = function dispatch(req, res, done) {
     }
 
     if (err) {
-      layer.handle_error(err, req, res, next)
+      return layer.handle_error(err, req, res, next)
     } else {
-      layer.handle_request(req, res, next)
+      return layer.handle_request(req, res, next)
     }
   }
 }

diff --git a/package.json b/package.json
@@ -16,6 +16,7 @@
   },
   "devDependencies": {
     "after": "0.8.1",
+    "bluebird": "3.0.6",
     "finalhandler": "0.4.0",
     "istanbul": "0.4.0",
     "mocha": "2.3.3",

diff --git a/test/router.js b/test/router.js
@@ -1,6 +1,7 @@
 
 var after = require('after')
 var methods = require('methods')
+var Bluebird = require('bluebird')
 var Router = require('..')
 var utils = require('./support/utils')
 
@@ -20,7 +21,7 @@ describe('Router', function () {
     assert.equal(typeof (new Router()), 'function')
   })
 
-  it('should reject missing callback', function () {
+  it.skip('should reject missing callback', function () {
     var router = new Router()
     assert.throws(router.bind(router, {}, {}), /argument callback is required/)
   })
@@ -539,6 +540,59 @@ describe('Router', function () {
       .expect(404, done)
     })
 
+    it('should handle downstream with promises', function (done) {
+      var router = new Router()
+      var server = createServer(router)
+      var message = 'hello world'
+
+      router.use(function (req, res, next) {
+        return next().then(res.end.bind(res))
+      })
+
+      router.use(function (req, res, next) {
+        return Bluebird.resolve(message)
+      })
+
+      request(server)
+      .get('/')
+      .expect(200, message, done)
+    })
+
+    if (typeof Promise === 'function') {
+      it('should always expose promises through `next`', function (done) {
+        var router = new Router()
+        var server = createServer(router)
+
+        router.use(function (req, res, next) {
+          var result = next()
+          assert(result && typeof result.then === 'function', '`next` is promise')
+          return result
+        })
+
+        request(server)
+        .get('/')
+        .expect(404, done)
+      })
+
+      it('should coerce middleware result to promises', function (done) {
+        var router = new Router()
+        var server = createServer(router)
+        var message = 'hello world'
+
+        router.use(function (req, res, next) {
+          return next().then(res.end.bind(res))
+        })
+
+        router.use(function (req, res, next) {
+          return message
+        })
+
+        request(server)
+        .get('/')
+        .expect(200, message, done)
+      })
+    }
+
     describe('error handling', function () {
       it('should invoke error function after next(err)', function (done) {
         var router = new Router()
@@ -570,6 +624,40 @@ describe('Router', function () {
         .expect(200, 'saw Error: boom!', done)
       })
 
+      it('should invoke error function after rejected promise', function (done) {
+        var router = new Router()
+        var server = createServer(router)
+
+        router.use(function handle(req, res, next) {
+          return Bluebird.reject(new Error('boom!'))
+        })
+
+        router.use(sawError)
+
+        request(server)
+        .get('/')
+        .expect(200, 'saw Error: boom!', done)
+      })
+
+      it('should handle downstream errors with promises', function (done) {
+        var router = new Router()
+        var server = createServer(router)
+
+        router.use(function (req, res, next) {
+          return next().catch(function (err) {
+            return sawError(err, res, res, next)
+          })
+        })
+
+        router.use(function (req, res, next) {
+          return Bluebird.reject(new Error('boom!'))
+        })
+
+        request(server)
+        .get('/')
+        .expect(200, 'saw Error: boom!', done)
+      })
+
       it('should not invoke error functions above function', function (done) {
         var router = new Router()
         var server = createServer(router)

diff --git a/test/support/utils.js b/test/support/utils.js
@@ -16,7 +16,7 @@ function createHitHandle(num) {
   var name = 'x-fn-' + String(num)
   return function hit(req, res, next) {
     res.setHeader(name, 'hit')
-    next()
+    return next()
   }
 }