feat(middleware): Return a promise that resolves when middleware stack done

docs/scripting.md

@@ -705,6 +705,10 @@ Every middleware receives the same API signature of `context`, `next`, and
 `done`. Different kinds of middleware may receive different information in the
 `context` object. For more details, see the API for each type of middleware.
 
+Middleware execution returns a promise that resolves with the final `context`
+when the middleware stack completes. If the middleware stack throws an error,
+the promise will be rejected with the error and `context` at that point.
+
 ### Error Handling
 
 For synchronous middleware (never yields to the event loop), hubot will automatically catch errors and emit an an `error` event, just like in standard listeners. Hubot will also automatically call the most recent `done` callback to unwind the middleware stack. Asynchronous middleware should catch its own exceptions, emit an `error` event, and call `done`. Any uncaught exceptions will interrupt all execution of middleware completion callbacks.

package.json

@@ -1,6 +1,9 @@
 {
   "name": "hubot",
   "version": "0.0.0-development",
+  "standard": {
+     "env": [ "mocha" ]
+  },
   "publishConfig": {
     "tag": "next"
   },
@@ -23,6 +26,7 @@
     "cline": "^0.8.2",
     "coffee-script": "1.6.3",
     "connect-multiparty": "^1.2.5",
+    "es6-promise": "^4.1.0",
     "express": "^3.21.2",
     "log": "1.4.0",
     "optparse": "1.0.4",

src/middleware.js

@@ -22,43 +22,54 @@ class Middleware {
   // done() - Initial (final) completion callback. May be wrapped by
   //     executed middleware.
   //
-  // Returns nothing
+  // Returns promise - resolves with context when middleware completes
   // Returns before executing any middleware
   execute (context, next, done) {
-    const self = this
+    return new Promise((resolve, reject) => {
+      const self = this
 
-    if (done == null) {
-      done = function () {}
-    }
+      if (done == null) {
+        done = function () {}
+      }
 
-    // Execute a single piece of middleware and update the completion callback
-    // (each piece of middleware can wrap the 'done' callback with additional
-    // logic).
-    function executeSingleMiddleware (doneFunc, middlewareFunc, cb) {
-      // Match the async.reduce interface
-      function nextFunc (newDoneFunc) {
-        cb(null, newDoneFunc || doneFunc)
+      // Allow each middleware to resolve the promise early if it calls done()
+      const pieceDone = () => {
+        done()
+        resolve(context)
       }
 
-      // Catch errors in synchronous middleware
-      try {
-        middlewareFunc(context, nextFunc, doneFunc)
-      } catch (err) {
-        // Maintaining the existing error interface (Response object)
-        self.robot.emit('error', err, context.response)
-        // Forcibly fail the middleware and stop executing deeper
-        doneFunc()
+      // Execute a single piece of middleware and update the completion callback
+      // (each piece of middleware can wrap the 'done' callback with additional
+      // logic).
+      function executeSingleMiddleware (doneFunc, middlewareFunc, cb) {
+        // Match the async.reduce interface
+        function nextFunc (newDoneFunc) {
+          cb(null, newDoneFunc || doneFunc)
+        }
+
+        // Catch errors in synchronous middleware
+        try {
+          middlewareFunc(context, nextFunc, doneFunc)
+        } catch (err) {
+          // Maintaining the existing error interface (Response object)
+          self.robot.emit('error', err, context.response)
+          // Forcibly fail the middleware and stop executing deeper
+          doneFunc()
+          err.context = context
+          reject(err)
+        }
       }
-    }
 
-    // Executed when the middleware stack is finished
-    function allDone (_, finalDoneFunc) {
-      next(context, finalDoneFunc)
-    }
+      // Executed when the middleware stack is finished
+      function allDone (_, finalDoneFunc) {
+        next(context, finalDoneFunc)
+        resolve(context)
+      }
 
-    // Execute each piece of middleware, collecting the latest 'done' callback
-    // at each step.
-    process.nextTick(async.reduce.bind(null, this.stack, done, executeSingleMiddleware, allDone))
+      // Execute each piece of middleware, collecting the latest 'done' callback
+      // at each step.
+      process.nextTick(async.reduce.bind(null, this.stack, pieceDone, executeSingleMiddleware, allDone))
+    })
   }
 
   // Public: Registers new middleware

src/response.js

@@ -24,65 +24,65 @@ class Response {
   // strings - One or more strings to be posted. The order of these strings
   //           should be kept intact.
   //
-  // Returns nothing.
+  // Returns promise - resolves with context when middleware completes
   send (/* ...strings */) {
     const strings = [].slice.call(arguments)
-    this.runWithMiddleware.apply(this, ['send', { plaintext: true }].concat(strings))
+    return this.runWithMiddleware.apply(this, ['send', { plaintext: true }].concat(strings))
   }
 
   // Public: Posts an emote back to the chat source
   //
   // strings - One or more strings to be posted. The order of these strings
   //           should be kept intact.
   //
-  // Returns nothing.
+  // Returns promise - resolves with context when middleware completes
   emote (/* ...strings */) {
     const strings = [].slice.call(arguments)
-    this.runWithMiddleware.apply(this, ['emote', { plaintext: true }].concat(strings))
+    return this.runWithMiddleware.apply(this, ['emote', { plaintext: true }].concat(strings))
   }
 
   // Public: Posts a message mentioning the current user.
   //
   // strings - One or more strings to be posted. The order of these strings
   //           should be kept intact.
   //
-  // Returns nothing.
+  // Returns promise - resolves with context when middleware completes
   reply (/* ...strings */) {
     const strings = [].slice.call(arguments)
-    this.runWithMiddleware.apply(this, ['reply', { plaintext: true }].concat(strings))
+    return this.runWithMiddleware.apply(this, ['reply', { plaintext: true }].concat(strings))
   }
 
   // Public: Posts a topic changing message
   //
   // strings - One or more strings to set as the topic of the
   //           room the bot is in.
   //
-  // Returns nothing.
+  // Returns promise - resolves with context when middleware completes
   topic (/* ...strings */) {
     const strings = [].slice.call(arguments)
-    this.runWithMiddleware.apply(this, ['topic', { plaintext: true }].concat(strings))
+    return this.runWithMiddleware.apply(this, ['topic', { plaintext: true }].concat(strings))
   }
 
   // Public: Play a sound in the chat source
   //
   // strings - One or more strings to be posted as sounds to play. The order of
   //           these strings should be kept intact.
   //
-  // Returns nothing
+  // Returns promise - resolves with context when middleware completes
   play (/* ...strings */) {
     const strings = [].slice.call(arguments)
-    this.runWithMiddleware.apply(this, ['play'].concat(strings))
+    return this.runWithMiddleware.apply(this, ['play'].concat(strings))
   }
 
   // Public: Posts a message in an unlogged room
   //
   // strings - One or more strings to be posted. The order of these strings
   //           should be kept intact.
   //
-  // Returns nothing
+  // Returns promise - resolves with context when middleware completes
   locked (/* ...strings */) {
     const strings = [].slice.call(arguments)
-    this.runWithMiddleware.apply(this, ['locked', { plaintext: true }].concat(strings))
+    return this.runWithMiddleware.apply(this, ['locked', { plaintext: true }].concat(strings))
   }
 
   // Private: Call with a method for the given strings using response

src/robot.js

@@ -285,12 +285,12 @@ class Robot {
   //
   // cb - Optional callback that is called when message processing is complete
   //
-  // Returns nothing.
+  // Returns promise - resolves with context when middleware completes
   // Returns before executing callback
   receive (message, cb) {
     // When everything is finished (down the middleware stack and back up),
     // pass control back to the robot
-    this.middleware.receive.execute({ response: new Response(this, message) }, this.processListeners.bind(this), cb)
+    return this.middleware.receive.execute({ response: new Response(this, message) }, this.processListeners.bind(this), cb)
   }
 
   // Private: Passes the given message to any interested Listeners.

test/middleware_test.js

@@ -224,6 +224,70 @@ describe('Middleware', function () {
         )
       })
 
+      it('returns a promise that resolves when async middleware stack is complete', function (testDone) {
+        const testMiddlewareA = (context, next, done) => {
+          setTimeout(() => {
+            context.A = 'done'
+            next(done)
+          }, 50)
+        }
+
+        const testMiddlewareB = (context, next, done) => {
+          setTimeout(() => {
+            context.B = 'done'
+            next(done)
+          }, 50)
+        }
+
+        this.middleware.register(testMiddlewareA)
+        this.middleware.register(testMiddlewareB)
+
+        const middlewareFinished = () => {}
+
+        const middlewarePromise = this.middleware.execute(
+          {},
+          (_, done) => done(),
+          middlewareFinished
+        )
+
+        middlewarePromise.then((finalContext) => {
+          expect(finalContext).to.deep.equal({ A: 'done', B: 'done' })
+          testDone()
+        })
+      })
+
+      it('promise resolves when middleware completes early, with context at that point', function (testDone) {
+        const testMiddlewareA = (context, next, done) => {
+          setTimeout(() => {
+            context.A = 'done'
+            done()
+          }, 50)
+        }
+
+        const testMiddlewareB = (context, next, done) => {
+          setTimeout(() => {
+            context.B = 'done'
+            next(done)
+          }, 50)
+        }
+
+        this.middleware.register(testMiddlewareA)
+        this.middleware.register(testMiddlewareB)
+
+        const middlewareFinished = () => {}
+
+        const middlewarePromise = this.middleware.execute(
+          {},
+          (_, done) => done(),
+          middlewareFinished
+        )
+
+        middlewarePromise.then((finalContext) => {
+          expect(finalContext).to.deep.equal({ A: 'done' })
+          testDone()
+        })
+      })
+
       describe('error handling', function () {
         it('does not execute subsequent middleware after the error is thrown', function (testDone) {
           const middlewareExecution = []
@@ -258,7 +322,7 @@ describe('Middleware', function () {
             {},
             middlewareFinished,
             middlewareFailed
-          )
+          ).catch((reason) => {}) // supress unhandled promise rejection warning
         })
 
         it('emits an error event', function (testDone) {
@@ -287,7 +351,7 @@ describe('Middleware', function () {
             {response: testResponse},
             middlewareFinished,
             middlewareFailed
-          )
+          ).catch((reason) => {}) // supress unhandled promise rejection warning
         })
 
         it('unwinds the middleware stack (calling all done functions)', function (testDone) {
@@ -319,7 +383,7 @@ describe('Middleware', function () {
             {},
             middlewareFinished,
             middlewareFailed
-          )
+          ).catch((reason) => {}) // supress unhandled promise rejection warning
         })
       })
     })