Update rest of tests for new Middleware

lib/base_bot.js

@@ -618,20 +618,23 @@ class BaseBot extends EventEmitter {
    * @param {object} update
    */
   __emitUpdate(update) {
-    const incomingMiddlewarePromise = this.master
-      ? this.master.middleware.__runIncomingMiddleware(this, update)
-      : Promise.resolve(update);
+    if (!this.master) {
+      return Promise.reject(new Error('bot needs to be added to a botmaster ' +
+                            'instance in order to emit received updates'));
+    }
 
-    return incomingMiddlewarePromise
+    return this.master.middleware.__runIncomingMiddleware(this, update)
     .catch((err) => {
       // doing this, to make sure all errors (even ones rejected from
       // promises within incoming middleware) can be retrieved somewhere;
-      if (err.message) {
-        err.message = `"${err.message}". This is most probably on your end.`;
-      } else if (err === 'cancel') {
+      if (err === 'cancel') {
         return 'cancelled';
       }
-      this.emit('error', err);
+      if (err && err.message) {
+        err.message = `"${err.message}". This is most probably on your end.`;
+      }
+
+      this.emit('error', err || 'empty error object');
       return err;
     });
   }

lib/botmaster.js

@@ -190,23 +190,35 @@ class Botmaster extends EventEmitter {
    * Add middleware to this botmaster object
    * This function is just sugar for `middleware.__use` in them
    *
-   * @param {object} params
+   * @param {object} middleware
    *
-   * The params object is something that looks like this:
+   * The middleware object is something that looks like this for incoming:
+   * ```js
    * {
-   *  incoming: {
-   *    options: 'someOptionsObject',
-   *    controller: 'someCallbackFunction',
-   *  },
+   *  type: 'incoming',
+   *  controller: (bot, update, next) => {
+   *    // do stuff with update,
+   *    // call next (or return a promise)
+   *  }
    * }
+   * ```
+   *
+   * and like this for outgoing middleware
    *
-   * for incoming middleware. Write outgoing instead of incoming for outgoing
-   * middleware
+   * ```js
+   * {
+   *  type: 'outgoing',
+   *  controller: (bot, update, message, next) => {
+   *    // do stuff with message,
+   *    // call next (or return a promise)
+   *  }
+   * }
+   * ```
    *
    * @return {Botmaster} returns the botmaster object so you can chain middleware
    */
-  use(params) {
-    this.middleware.__use(params);
+  use(middleware) {
+    this.middleware.__use(middleware);
 
     return this;
   }
@@ -217,24 +229,15 @@ class Botmaster extends EventEmitter {
    * the outgoing middleware at end of outgoing stack.
    * This function is just sugar `middleware.useWrapped`.
    *
-   * @param {object} params
+   * @param {object} incomingMiddleware
+   * @param {object} outgoingMiddleware
    *
-   * The params object is something that looks like this:
-   * {
-   *  incoming: {
-   *    options: 'someOptionsObject',
-   *    controller: 'someCallbackFunction',
-   *  },
-   *  outgoing: {
-   *    options: 'someOptionsObject',
-   *    controller: 'someCallbackFunction'
-   *  }
-   * }
+   * The middleware objects are as you'd expect them to be (see use)
    *
    * @return {Botmaster} returns the botmaster object so you can chain middleware
    */
-  useWrapped(params) {
-    this.middleware.__useWrapped(params);
+  useWrapped(incomingMiddleware, outgoingMiddleware) {
+    this.middleware.__useWrapped(incomingMiddleware, outgoingMiddleware);
 
     return this;
   }

lib/middleware.js

@@ -13,7 +13,6 @@ class Middleware {
    * @ignore
    */
   constructor() {
-    this.incomingMiddlewareStack = [];
     this.incomingMiddlewareStack = [];
     this.outgoingMiddlewareStack = [];
   }
@@ -43,15 +42,19 @@ class Middleware {
    * @param {object} params
    */
   __useWrapped(incomingMiddleware, outgoingMiddleware) {
+    if (!incomingMiddleware || !outgoingMiddleware) {
+      throw new Error('useWrapped should be called with both an' +
+                      ' incoming and an outgoing middleware');
+    }
     this.__validateMiddleware(incomingMiddleware);
     this.__validateMiddleware(outgoingMiddleware);
 
     if (incomingMiddleware.type === 'outgoing') {
       throw new TypeError('first argument of "useWrapped" should be an' +
-      'incoming middleware');
+      ' incoming middleware');
     } else if (outgoingMiddleware.type === 'incoming') {
       throw new TypeError('second argument of "useWrapped" should be an' +
-      'outgoing middleware');
+      ' outgoing middleware');
     }
 
     this.incomingMiddlewareStack.unshift(incomingMiddleware);
@@ -63,7 +66,7 @@ class Middleware {
 
   __validateMiddleware(middleware) {
     if (typeof middleware !== 'object') {
-      throw new Error(`Can't add middleware of type ${typeof middleware}`);
+      throw new Error(`middleware should be an object. Not ${typeof middleware}`);
     }
 
     const middlewareController = middleware.controller;
@@ -143,10 +146,12 @@ class Middleware {
                                 'returns a promise/is an async function');
               } else if (err.err) {
                 throw err.err;
+              } else {
+                return;
               }
-            } else if (err) {
-              throw err;
             }
+
+            throw err;
           });
         }
         //  otherwise, return nothing

tests/botmaster/add_bot.js

@@ -45,12 +45,15 @@ const arbitraryBotMacro = (t, botSettings) => {
 
       request(requestOptions);
 
-      botmaster.on('update', (onUpdateBot, update) => {
-        t.deepEqual(update.raw, updateToSend);
-        botmaster.server.close(resolve);
+      botmaster.use({
+        type: 'incoming',
+        controller: (onUpdateBot, update) => {
+          t.deepEqual(update.raw, updateToSend);
+          botmaster.server.close(resolve);
+        },
       });
 
-      botmaster.on('error', (err) => {
+      botmaster.on('error', () => {
         botmaster.server.close(resolve);
       });
     });
@@ -135,26 +138,29 @@ test('works with an express server AND both an express and a koa bot', (t) => {
 
       // catch update events
       let receivedUpdatesCount = 0;
-      botmaster.on('update', (onUpdateBot, update) => {
-        receivedUpdatesCount += 1;
-        if (update.raw.text.indexOf('Koa') > -1) {
-          t.deepEqual(update.raw, updateToSendToKoaBot);
-        } else if (update.raw.text.indexOf('express') > -1) {
-          t.deepEqual(update.raw, updateToSendToExpressBot);
-        }
-        if (receivedUpdatesCount === 2) {
-          const appRequestOptions = {
-            uri: 'http://localhost:3000/someRoute',
-            json: true,
-          };
-          request.get(appRequestOptions)
-
-          .then((body) => {
-            t.deepEqual(appResponse, body);
-            t.is(botmaster.server, myServer);
-            botmaster.server.close(resolve);
-          });
-        }
+      botmaster.use({
+        type: 'incoming',
+        controller: ('update', (onUpdateBot, update) => {
+          receivedUpdatesCount += 1;
+          if (update.raw.text.indexOf('Koa') > -1) {
+            t.deepEqual(update.raw, updateToSendToKoaBot);
+          } else if (update.raw.text.indexOf('express') > -1) {
+            t.deepEqual(update.raw, updateToSendToExpressBot);
+          }
+          if (receivedUpdatesCount === 2) {
+            const appRequestOptions = {
+              uri: 'http://localhost:3000/someRoute',
+              json: true,
+            };
+            request.get(appRequestOptions)
+
+            .then((body) => {
+              t.deepEqual(appResponse, body);
+              t.is(botmaster.server, myServer);
+              botmaster.server.close(resolve);
+            });
+          }
+        }),
       });
       // ////////////////////////////
     });

tests/middleware/use.js

@@ -33,6 +33,18 @@ test.afterEach((t) => {
   });
 });
 
+test('throws an error middleware is not an object', (t) => {
+  t.plan(1);
+
+  try {
+    t.context.botmaster.use('something');
+  } catch (err) {
+    t.is(err.message,
+      'middleware should be an object. Not string',
+      'Error message is not the same as expected');
+  }
+});
+
 test('throws an error if type is not incoming or outgoing', (t) => {
   t.plan(1);
 
@@ -151,7 +163,70 @@ test('using async function', incomingMiddlewareErrorMacro,
     update.blop();
   });
 
-test('Throws error if next is used within returned promise', (t) => {
+test('Error is emitted if error is thrown by user and does not inherit from Error', (t) => {
+  t.plan(1);
+
+  return new Promise((resolve) => {
+    const botmaster = t.context.botmaster;
+
+    botmaster.use({
+      controller: async () => {
+        const err = 'not expected';
+        throw err;
+      },
+      type: 'incoming',
+    });
+
+    botmaster.use({
+      type: 'incoming',
+      controller: () => {
+        t.fail('this middleware should not get hit');
+        resolve();
+      },
+    });
+
+    botmaster.on('error', (bot, err) => {
+      t.is(err,
+        'not expected',
+        'Error message did not match');
+      resolve();
+    });
+
+    request(t.context.baseRequestOptions);
+  });
+});
+
+test('Error is emitted if error is thrown by user and is falsy', (t) => {
+  t.plan(1);
+
+  return new Promise((resolve) => {
+    const botmaster = t.context.botmaster;
+
+    botmaster.use({
+      controller: () => Promise.reject(),
+      type: 'incoming',
+    });
+
+    botmaster.use({
+      type: 'incoming',
+      controller: () => {
+        t.fail('this middleware should not get hit');
+        resolve();
+      },
+    });
+
+    botmaster.on('error', (bot, err) => {
+      t.is(err,
+        'empty error object',
+        'Error message did not match');
+      resolve();
+    });
+
+    request(t.context.baseRequestOptions);
+  });
+});
+
+test('Emits error if next is used within returned promise', (t) => {
   t.plan(1);
 
   return new Promise((resolve) => {