feat: v9 - async/await (#1664)

* Working through the blast radius * doesn't make sense to have a didMatchCallback another, especially since the Response object has it in it's match property * all tests are passing * applying async/await in other places * fix the fixtures * Removing module dependency * Update supported node engine to LTS version * npm audit fix * Set the version to 9.0.0.
hubotio · Sep 16, 2023 · a4c2ec8 · a4c2ec8
1 parent bdebbb9
commit a4c2ec8
Show file tree

Hide file tree

Showing 17 changed files with 441 additions and 1,077 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "hubot",
-  "version": "0.0.0-development",
+  "version": "9.0.0",
   "author": "hubot",
   "keywords": [
     "github",
@@ -15,7 +15,6 @@
     "url": "https://github.com/hubotio/hubot.git"
   },
   "dependencies": {
-    "async": "^3.2.4",
     "cline": "^0.8.2",
     "coffeescript": "^2.7.0",
     "connect-multiparty": "^2.2.0",
@@ -34,8 +33,8 @@
     "standard": "^17.1.0"
   },
   "engines": {
-    "node": "> 4.0.0",
-    "npm": "> 2.0.0"
+    "node": "> 16.20.2",
+    "npm": "> 8.19.4"
   },
   "main": "./index",
   "bin": {

diff --git a/src/adapter.js b/src/adapter.js
@@ -16,19 +16,18 @@ class Adapter extends EventEmitter {
   // envelope - A Object with message, room and user details.
   // strings  - One or more Strings for each message to send.
   //
-  // Returns nothing.
-  send (envelope/* , ...strings */) {}
+  // Returns results from adapter.
+  async send (envelope, ...strings) {}
 
   // Public: Raw method for sending emote data back to the chat source.
   // Defaults as an alias for send
   //
   // envelope - A Object with message, room and user details.
   // strings  - One or more Strings for each message to send.
   //
-  // Returns nothing.
-  emote (envelope/* , ...strings */) {
-    const strings = [].slice.call(arguments, 1)
-    return this.send.apply(this, [envelope].concat(strings))
+  // Returns results from adapter.
+  async emote (envelope, ...strings) {
+    return this.senda(envelope, ...strings)
   }
 
   // Public: Raw method for building a reply and sending it back to the chat
@@ -37,24 +36,24 @@ class Adapter extends EventEmitter {
   // envelope - A Object with message, room and user details.
   // strings  - One or more Strings for each reply to send.
   //
-  // Returns nothing.
-  reply (envelope/* , ...strings */) {}
+  // Returns results from adapter.
+  async reply (envelope, ...strings) {}
 
   // Public: Raw method for setting a topic on the chat source. Extend this.
   //
   // envelope - A Object with message, room and user details.
   // strings  - One more more Strings to set as the topic.
   //
-  // Returns nothing.
-  topic (envelope/* , ...strings */) {}
+  // Returns results from adapter.
+  async topic (envelope, ...strings) {}
 
   // Public: Raw method for playing a sound in the chat source. Extend this.
   //
   // envelope - A Object with message, room and user details.
   // strings  - One or more strings for each play message to send.
   //
-  // Returns nothing
-  play (envelope/* , ...strings */) {}
+  // Returns results from adapter.
+  async play (envelope, ...strings) {}
 
   // Public: Raw method for invoking the bot to run. Extend this.
   //
@@ -69,8 +68,8 @@ class Adapter extends EventEmitter {
   // Public: Dispatch a received message to the robot.
   //
   // Returns nothing.
-  receive (message) {
-    this.robot.receive(message)
+  async receive (message) {
+    await this.robot.receive(message)
   }
 
   // Public: Get an Array of User objects stored in the brain.

diff --git a/src/adapters/shell.js b/src/adapters/shell.js
@@ -22,21 +22,17 @@ class Shell extends Adapter {
     this.name = 'Shell'
   }
 
-  send (envelope/* , ...strings */) {
-    const strings = [].slice.call(arguments, 1)
-
+  async send (envelope, ...strings) {
     Array.from(strings).forEach(str => console.log(bold(str)))
   }
 
-  emote (envelope/* , ...strings */) {
-    const strings = [].slice.call(arguments, 1)
+  async emote (envelope, ...strings) {
     Array.from(strings).map(str => this.send(envelope, `* ${str}`))
   }
 
-  reply (envelope/* , ...strings */) {
-    const strings = [].slice.call(arguments, 1).map((s) => `${envelope.user.name}: ${s}`)
-
-    this.send.apply(this, [envelope].concat(strings))
+  async reply (envelope, ...strings) {
+    strings = strings.map((s) => `${envelope.user.name}: ${s}`)
+    this.send(envelope, ...strings)
   }
 
   run () {

diff --git a/src/listener.js b/src/listener.js
@@ -20,19 +20,19 @@ class Listener {
   constructor (robot, matcher, options, callback) {
     this.robot = robot
     this.matcher = matcher
-    this.options = options
+    this.options = options ?? {}
     this.callback = callback
 
     if (this.matcher == null) {
       throw new Error('Missing a matcher for Listener')
     }
 
-    if (this.callback == null) {
+    if (!this.callback) {
       this.callback = this.options
       this.options = {}
     }
 
-    if (this.options.id == null) {
+    if (!this.options?.id) {
       this.options.id = null
     }
 
@@ -49,60 +49,40 @@ class Listener {
   //
   // message - A Message instance.
   // middleware - Optional Middleware object to execute before the Listener callback
-  // callback - Optional Function called with a boolean of whether the matcher matched
   //
-  // Returns a boolean of whether the matcher matched.
-  // Returns before executing callback
-  call (message, middleware, didMatchCallback) {
-    // middleware argument is optional
-    if (didMatchCallback == null && typeof middleware === 'function') {
-      didMatchCallback = middleware
-      middleware = undefined
+  // Returns the result of the callback.
+  async call (message, middleware) {
+    if (middleware && typeof middleware === 'function') {
+      const fn = middleware
+      middleware = new Middleware(this.robot)
+      middleware.register(fn)
     }
 
-    // ensure we have a Middleware object
-    if (middleware == null) {
+    if (!middleware) {
       middleware = new Middleware(this.robot)
     }
 
     const match = this.matcher(message)
-    if (match) {
-      if (this.regex) {
-        this.robot.logger.debug(`Message '${message}' matched regex /${inspect(this.regex)}/; listener.options = ${inspect(this.options)}`)
-      }
-
-      // special middleware-like function that always executes the Listener's
-      // callback and calls done (never calls 'next')
-      const executeListener = (context, done) => {
-        this.robot.logger.debug(`Executing listener callback for Message '${message}'`)
-        try {
-          this.callback(context.response)
-        } catch (err) {
-          this.robot.emit('error', err, context.response)
-        }
-        done()
-      }
+    if (!match) return null
+    if (this.regex) {
+      this.robot.logger.debug(`Message '${message}' matched regex /${inspect(this.regex)}/; listener.options = ${inspect(this.options)}`)
+    }
 
-      // When everything is finished (down the middleware stack and back up),
-      // pass control back to the robot
-      const allDone = function allDone () {
-        // Yes, we tried to execute the listener callback (middleware may
-        // have intercepted before actually executing though)
-        if (didMatchCallback != null) {
-          process.nextTick(() => didMatchCallback(true))
-        }
-      }
+    const response = new this.robot.Response(this.robot, message, match)
 
-      const response = new this.robot.Response(this.robot, message, match)
-      middleware.execute({ listener: this, response }, executeListener, allDone)
-      return true
-    } else {
-      if (didMatchCallback != null) {
-        // No, we didn't try to execute the listener callback
-        process.nextTick(() => didMatchCallback(false))
-      }
-      return false
+    try {
+      const shouldContinue = await middleware.execute({ listener: this, response })
+      if (shouldContinue === false) return null
+    } catch (e) {
+      this.robot.logger.error(`Error executing middleware for listener: ${e.stack}`)
+    }
+    try {
+      return await this.callback(response)
+    } catch (e) {
+      this.robot.logger.error(`Error executing listener callback: ${e.stack}`)
+      this.robot.emit('error', e, response)
     }
+    return null
   }
 }
 

diff --git a/src/message.js b/src/message.js
@@ -7,7 +7,7 @@ class Message {
   constructor (user, done) {
     this.user = user
     this.done = done || false
-    this.room = this.user.room
+    this.room = this.user?.room
   }
 
   // Indicates that no other Listener should be called on this object

diff --git a/src/middleware.js b/src/middleware.js
@@ -1,7 +1,5 @@
 'use strict'
 
-const async = require('async')
-
 class Middleware {
   constructor (robot) {
     this.robot = robot
@@ -16,68 +14,29 @@ class Middleware {
   // context - context object that is passed through the middleware stack.
   //     When handling errors, this is assumed to have a `response` property.
   //
-  // next(context, done) - Called when all middleware is complete (assuming
-  //     all continued by calling respective 'next' functions)
-  //
-  // done() - Initial (final) completion callback. May be wrapped by
-  //     executed middleware.
-  //
-  // Returns nothing
-  // Returns before executing any middleware
-  execute (context, next, done) {
-    const self = this
-
-    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)
-      }
-
-      // Catch errors in synchronous middleware
+  // Returns bool, true | false, whether or not to continue execution
+  async execute (context) {
+    let shouldContinue = true
+    for await (const middleware of this.stack) {
       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()
+        shouldContinue = await middleware(context)
+        if (shouldContinue === false) break
+      } catch (e) {
+        this.robot.emit('error', e, context.response)
+        break
       }
     }
-
-    // Executed when the middleware stack is finished
-    function allDone (_, finalDoneFunc) {
-      next(context, finalDoneFunc)
-    }
-
-    // Execute each piece of middleware, collecting the latest 'done' callback
-    // at each step.
-    process.nextTick(async.reduce.bind(null, this.stack, done, executeSingleMiddleware, allDone))
+    return shouldContinue
   }
 
   // Public: Registers new middleware
   //
-  // middleware - A generic pipeline component function that can either
-  //              continue the pipeline or interrupt it. The function is called
-  //              with (robot, context, next, done). If execution should
-  //              continue (next middleware, final callback), the middleware
-  //              should call the 'next' function with 'done' as an optional
-  //              argument.
-  //              If not, the middleware should call the 'done' function with
-  //              no arguments. Middleware may wrap the 'done' function in
-  //              order to execute logic after the final callback has been
-  //              executed.
+  // middleware - Middleware function to execute prior to the listener callback. Return false to prevent execution of the listener callback.
   //
   // Returns nothing.
   register (middleware) {
-    if (middleware.length !== 3) {
-      throw new Error(`Incorrect number of arguments for middleware callback (expected 3, got ${middleware.length})`)
+    if (middleware.length !== 1) {
+      throw new Error(`Incorrect number of arguments for middleware callback (expected 1, got ${middleware.length})`)
     }
     this.stack.push(middleware)
   }