Handle promise errors in router middleware

* Add lots of `return` calls for proxying the function returns back to the previous middleware stack function
* Catch promise errors and pass to `next`

index.js

@@ -28,11 +28,6 @@ var setPrototypeOf = require('setprototypeof')
 
 var slice = Array.prototype.slice
 
-/* istanbul ignore next */
-var defer = typeof setImmediate === 'function'
-  ? setImmediate
-  : function(fn){ process.nextTick(fn.bind.apply(fn, arguments)) }
-
 /**
  * Expose `Router`.
  */
@@ -61,7 +56,7 @@ function Router(options) {
   var opts = options || {}
 
   function router(req, res, next) {
-    router.handle(req, res, next)
+    return router.handle(req, res, next)
   }
 
   // inherit from the correct prototype
@@ -186,7 +181,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 +203,7 @@ Router.prototype.handle = function handle(req, res, callback) {
 
     // no more matching layers
     if (idx >= stack.length) {
-      defer(done, layerError)
-      return
+      return done(layerError)
     }
 
     // get pathname of request
@@ -281,7 +275,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,16 +284,15 @@ 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)
     })
   }
 
   function trim_prefix(layer, layerError, layerPath, path) {
     var c = path[layerPath.length]
 
     if (c && c !== '/') {
-      next(layerError)
-      return
+      return next(layerError)
     }
 
      // Trim off the part of the url that matches the route
@@ -324,9 +317,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)
     }
   }
 }
@@ -399,7 +392,7 @@ Router.prototype.process_params = function process_params(layer, called, req, re
       value: paramVal
     }
 
-    paramCallback()
+    return paramCallback()
   }
 
   // single param callbacks
@@ -412,20 +405,19 @@ Router.prototype.process_params = function process_params(layer, called, req, re
     if (err) {
       // store error
       paramCalled.error = err
-      param(err)
-      return
+      return param(err)
     }
 
     if (!fn) return param()
 
     try {
-      fn(req, res, paramCallback, paramVal, key.name)
+      return fn(req, res, paramCallback, paramVal, key.name)
     } catch (e) {
-      paramCallback(e)
+      return paramCallback(e)
     }
   }
 
-  param()
+  return param()
 }
 
 /**

lib/layer.js

@@ -15,6 +15,20 @@
 var pathRegexp = require('path-to-regexp')
 var debug = require('debug')('router:layer')
 
+/**
+ * Normalize the middleware result with promise error handling.
+ */
+
+function normalize (result, next) {
+  if (result && typeof result.then === 'function') {
+    return result.then(null, function (err) {
+      return next(err || new Error('Promise was rejected with a falsy value'))
+    })
+  }
+
+  return result
+}
+
 /**
  * Module variables.
  * @private
@@ -66,9 +80,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 +104,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)
   }
 }
 

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)
     }
   }
 }

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",

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')
 
@@ -539,6 +540,24 @@ describe('Router', function () {
       .expect(404, done)
     })
 
+    it('should read downstream results', 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)
+    })
+
     describe('error handling', function () {
       it('should invoke error function after next(err)', function (done) {
         var router = new Router()
@@ -570,6 +589,21 @@ 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 not invoke error functions above function', function (done) {
         var router = new Router()
         var server = createServer(router)

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()
   }
 }