Fix Node v6.6.0+ UnhandledPromiseRejectionWarning

Backported from mbland/hubot-slack-github-issues#7.

As discovered in 18F/hubot-slack-github-issues#51, the
`UnhandledPromiseRejectionWarning` and `PromiseRejectionHandledWarning`
warnings were apparently added in v6.6.0
(https://nodejs.org/en/blog/release/v6.6.0/); specifically it was added
by nodejs/node#8223. See also:

  nodejs/node#6439
  nodejs/node#8217
  https://nodejs.org/dist/latest-v6.x/docs/api/process.html#process_event_unhandledrejection
  https://nodejs.org/dist/latest-v6.x/docs/api/process.html#process_event_rejectionhandled
  https://nodejs.org/dist/latest-v6.x/docs/api/process.html#process_event_warning

A test failure from `solutions/06-integration/test/integration-test.js`
after upgrading to Node v6.9.1 showed output such as:

```
-  "(node:87412) UnhandledPromiseRejectionWarning: Unhandled
     promise rejection (rejection id: 14): Error: failed to create a
     GitHub issue in 18F/handbook: received 500 response from GitHub
     API: {\"message\":\"test failure\"}\n"
```

This was happening because `scripts/slack-github-issues.js`
ignored the return value from `Middleware.execute()`, whether it was
undefined or a `Promise`. For consistency's sake (and to provide a
clearer upgrade path to the current state of
mbland/slack-github-issues), `Middleware.execute()` always returns a
`Promise`, and `scripts/slack-github-issues.js` handles and ignores any
rejected Promises.

This fixed the `integration-test.js` error, but also required minor
updates to `solutions/{05-middleware,complete}/test/middleware-test.js`.
The next commit will update the tutorial narrative to account for this
change.

solutions/05-middleware/lib/middleware.js

@@ -28,7 +28,8 @@ Middleware.prototype.execute = function(context, next, done) {
         JSON.stringify(context.response.message.rawMessage, null, 2);
     this.logger.error(null, errorMessage);
     context.response.reply(errorMessage);
-    return next(done);
+    next(done);
+    return Promise.reject();
   }
 };
 
@@ -40,13 +41,15 @@ function doExecute(middleware, context, next, done) {
       finish;
 
   if (!rule) {
-    return next(done);
+    next(done);
+    return Promise.reject();
   }
 
   msgId = messageId(message);
   if (middleware.inProgress[msgId]) {
     middleware.logger.info(msgId, 'already in progress');
-    return next(done);
+    next(done);
+    return Promise.reject();
   }
   middleware.inProgress[msgId] = true;
 

solutions/05-middleware/test/middleware-test.js

@@ -142,29 +142,30 @@ describe('Middleware', function() {
 
     it('should ignore messages that do not match', function() {
       delete context.response.message.rawMessage;
-      expect(middleware.execute(context, next, hubotDone)).to.be.undefined;
-      next.calledWith(hubotDone).should.be.true;
+      middleware.execute(context, next, hubotDone)
+        .should.be.rejectedWith(undefined).then(function() {
+          next.calledWith(hubotDone).should.be.true;
+        });
     });
 
     it('should not file another issue for the same message when ' +
       'one is in progress', function() {
       var result;
 
       result = middleware.execute(context, next, hubotDone);
-      expect(middleware.execute(context, next, hubotDone)).to.eql(undefined,
-        'middleware.execute did not prevent filing a second issue ' +
-        'when one was already in progress');
-
-      return result.should.become(helpers.ISSUE_URL).then(function() {
-        logger.info.args.should.include.something.that.deep.equals(
-          helpers.logArgs('already in progress'));
-
-        // Make another call to ensure that the ID is cleaned up. Normally the
-        // message will have a successReaction after the first successful
-        // request, but we'll test that in another case.
-        return middleware.execute(context, next, hubotDone)
-          .should.become(helpers.ISSUE_URL);
-      });
+      return middleware.execute(context, next, hubotDone)
+        .should.be.rejectedWith(undefined).then(function() {
+          logger.info.args.should.include.something.that.deep.equals(
+            helpers.logArgs('already in progress'));
+          next.calledWith(hubotDone).should.be.true;
+          return result.should.become(helpers.ISSUE_URL).then(function() {
+            // Make another call to ensure that the ID is cleaned up. Normally
+            // the  message will have a successReaction after the first
+            // successful request, but we'll test that in another case.
+            return middleware.execute(context, next, hubotDone)
+              .should.become(helpers.ISSUE_URL);
+          });
+        });
     });
 
     it('should not file another issue for the same message when ' +
@@ -250,7 +251,8 @@ describe('Middleware', function() {
             JSON.stringify(helpers.reactionAddedMessage(), null, 2);
 
       slackClient.getChannelName.throws();
-      expect(middleware.execute(context, next, hubotDone)).to.be.undefined;
+      middleware.execute(context, next, hubotDone)
+        .should.be.rejectedWith(undefined);
       next.calledWith(hubotDone).should.be.true;
       context.response.reply.args.should.eql([[errorMessage]]);
       logger.error.args.should.eql([[null, errorMessage]]);

solutions/06-integration/lib/middleware.js

@@ -28,7 +28,8 @@ Middleware.prototype.execute = function(context, next, done) {
         JSON.stringify(context.response.message.rawMessage, null, 2);
     this.logger.error(null, errorMessage);
     context.response.reply(errorMessage);
-    return next(done);
+    next(done);
+    return Promise.reject();
   }
 };
 
@@ -40,13 +41,15 @@ function doExecute(middleware, context, next, done) {
       finish;
 
   if (!rule) {
-    return next(done);
+    next(done);
+    return Promise.reject();
   }
 
   msgId = messageId(message);
   if (middleware.inProgress[msgId]) {
     middleware.logger.info(msgId, 'already in progress');
-    return next(done);
+    next(done);
+    return Promise.reject();
   }
   middleware.inProgress[msgId] = true;
 

solutions/06-integration/scripts/slack-github-issues.js

@@ -25,7 +25,7 @@ module.exports = function(robot) {
       logger);
 
     middleware = function(context, next, done) {
-      impl.execute(context, next, done);
+      impl.execute(context, next, done).catch(function() { });
     };
     middleware.impl = impl;
     robot.receiveMiddleware(middleware);

solutions/07-system/lib/middleware.js

@@ -28,7 +28,8 @@ Middleware.prototype.execute = function(context, next, done) {
         JSON.stringify(context.response.message.rawMessage, null, 2);
     this.logger.error(null, errorMessage);
     context.response.reply(errorMessage);
-    return next(done);
+    next(done);
+    return Promise.reject();
   }
 };
 
@@ -40,13 +41,15 @@ function doExecute(middleware, context, next, done) {
       finish;
 
   if (!rule) {
-    return next(done);
+    next(done);
+    return Promise.reject();
   }
 
   msgId = messageId(message);
   if (middleware.inProgress[msgId]) {
     middleware.logger.info(msgId, 'already in progress');
-    return next(done);
+    next(done);
+    return Promise.reject();
   }
   middleware.inProgress[msgId] = true;
 

solutions/07-system/scripts/slack-github-issues.js

@@ -25,7 +25,7 @@ module.exports = function(robot) {
       logger);
 
     middleware = function(context, next, done) {
-      impl.execute(context, next, done);
+      impl.execute(context, next, done).catch(function() { });
     };
     middleware.impl = impl;
     robot.receiveMiddleware(middleware);

solutions/complete/lib/middleware.js

@@ -28,7 +28,8 @@ Middleware.prototype.execute = function(context, next, done) {
         JSON.stringify(context.response.message.rawMessage, null, 2);
     this.logger.error(null, errorMessage);
     context.response.reply(errorMessage);
-    return next(done);
+    next(done);
+    return Promise.reject();
   }
 };
 
@@ -40,13 +41,15 @@ function doExecute(middleware, context, next, done) {
       finish;
 
   if (!rule) {
-    return next(done);
+    next(done);
+    return Promise.reject();
   }
 
   msgId = messageId(message);
   if (middleware.inProgress[msgId]) {
     middleware.logger.info(msgId, 'already in progress');
-    return next(done);
+    next(done);
+    return Promise.reject();
   }
   middleware.inProgress[msgId] = true;
 

solutions/complete/scripts/slack-github-issues.js

@@ -25,7 +25,7 @@ module.exports = function(robot) {
       logger);
 
     middleware = function(context, next, done) {
-      impl.execute(context, next, done);
+      impl.execute(context, next, done).catch(function() { });
     };
     middleware.impl = impl;
     robot.receiveMiddleware(middleware);

solutions/complete/test/middleware-test.js

@@ -142,29 +142,30 @@ describe('Middleware', function() {
 
     it('should ignore messages that do not match', function() {
       delete context.response.message.rawMessage;
-      expect(middleware.execute(context, next, hubotDone)).to.be.undefined;
-      next.calledWith(hubotDone).should.be.true;
+      middleware.execute(context, next, hubotDone)
+        .should.be.rejectedWith(undefined).then(function() {
+          next.calledWith(hubotDone).should.be.true;
+        });
     });
 
     it('should not file another issue for the same message when ' +
       'one is in progress', function() {
       var result;
 
       result = middleware.execute(context, next, hubotDone);
-      expect(middleware.execute(context, next, hubotDone)).to.eql(undefined,
-        'middleware.execute did not prevent filing a second issue ' +
-        'when one was already in progress');
-
-      return result.should.become(helpers.ISSUE_URL).then(function() {
-        logger.info.args.should.include.something.that.deep.equals(
-          helpers.logArgs('already in progress'));
-
-        // Make another call to ensure that the ID is cleaned up. Normally the
-        // message will have a successReaction after the first successful
-        // request, but we'll test that in another case.
-        return middleware.execute(context, next, hubotDone)
-          .should.become(helpers.ISSUE_URL);
-      });
+      return middleware.execute(context, next, hubotDone)
+        .should.be.rejectedWith(undefined).then(function() {
+          logger.info.args.should.include.something.that.deep.equals(
+            helpers.logArgs('already in progress'));
+          next.calledWith(hubotDone).should.be.true;
+          return result.should.become(helpers.ISSUE_URL).then(function() {
+            // Make another call to ensure that the ID is cleaned up. Normally
+            // the  message will have a successReaction after the first
+            // successful request, but we'll test that in another case.
+            return middleware.execute(context, next, hubotDone)
+              .should.become(helpers.ISSUE_URL);
+          });
+        });
     });
 
     it('should not file another issue for the same message when ' +
@@ -250,7 +251,8 @@ describe('Middleware', function() {
             JSON.stringify(helpers.reactionAddedMessage(), null, 2);
 
       slackClient.getChannelName.throws();
-      expect(middleware.execute(context, next, hubotDone)).to.be.undefined;
+      middleware.execute(context, next, hubotDone)
+        .should.be.rejectedWith(undefined);
       next.calledWith(hubotDone).should.be.true;
       context.response.reply.args.should.eql([[errorMessage]]);
       logger.error.args.should.eql([[null, errorMessage]]);