Drop next() as a function parameter type on listeners

[M1kep]

Description

Describe your issue here.

What type of issue is this? (place an x in one of the [ ])

• bug
• enhancement (feature request)
• question
• documentation related
• example code related
• testing related
• discussion

Requirements (place an x in each of the [ ])

• I've read and understood the Contributing guidelines and have done my best effort to follow them.
• I've read and agree to the Code of Conduct.
• I've searched for any related issues and avoided creating a duplicate issue.

---

Bug Report

Filling out the following details about bugs will help us solve your issue sooner.

Reproducible in:

package version: 3.11.0

node version: 16.15.0

OS version(s): Docker - node:16-alpine

Steps to reproduce:

1. Create an action listener on an App instance and call next()

            // Log action_ids of all actions when in Debug mode
            bot.appInstance?.action(/.*/, async ({ action, logger, next }) => {
                if ('action_id' in action) {
                    logger.debug(`Action Hit(${botName}): ${action.action_id}`);
                }
                await next();
            });

Expected result:

Either TypeScript does not provide next as a valid parameter, or next to exist.

Actual result:

Error:

[ERROR]  bolt-app UnknownError: next is not a function
    at asCodedError (/workspace/.yarn/cache/@slack-bolt-npm-3.11.0-1183767d75-25491d656e.zip/node_modules/@slack/bolt/src/errors.ts:56:10)
    at App.handleError (/workspace/.yarn/cache/@slack-bolt-npm-3.11.0-1183767d75-25491d656e.zip/node_modules/@slack/bolt/src/App.ts:1049:37)
    at App.processEvent (/workspace/.yarn/cache/@slack-bolt-npm-3.11.0-1183767d75-25491d656e.zip/node_modules/@slack/bolt/src/App.ts:1032:19)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async SocketModeClient.<anonymous> (/workspace/.yarn/cache/@slack-bolt-npm-3.11.0-1183767d75-25491d656e.zip/node_modules/@slack/bolt/src/receivers/SocketModeReceiver.ts:218:9) {
  code: 'slack_bolt_unknown_error',
  original: TypeError: next is not a function
      at /workspace/src/core/botManager.ts:64:23
      at /workspace/.yarn/cache/@slack-bolt-npm-3.11.0-1183767d75-25491d656e.zip/node_modules/@slack/bolt/src/App.ts:1006:27
      at invokeMiddleware (/workspace/.yarn/cache/@slack-bolt-npm-3.11.0-1183767d75-25491d656e.zip/node_modules/@slack/bolt/src/middleware/process.ts:32:12)
      at next (/workspace/.yarn/cache/@slack-bolt-npm-3.11.0-1183767d75-25491d656e.zip/node_modules/@slack/bolt/src/middleware/process.ts:24:21)
      at Array.<anonymous> (/workspace/.yarn/cache/@slack-bolt-npm-3.11.0-1183767d75-25491d656e.zip/node_modules/@slack/bolt/src/middleware/builtin.ts:197:11)
      at invokeMiddleware (/workspace/.yarn/cache/@slack-bolt-npm-3.11.0-1183767d75-25491d656e.zip/node_modules/@slack/bolt/src/middleware/process.ts:23:47)
      at next (/workspace/.yarn/cache/@slack-bolt-npm-3.11.0-1183767d75-25491d656e.zip/node_modules/@slack/bolt/src/middleware/process.ts:24:21)
      at Array.onlyActions (/workspace/.yarn/cache/@slack-bolt-npm-3.11.0-1183767d75-25491d656e.zip/node_modules/@slack/bolt/src/middleware/builtin.ts:42:9)
      at invokeMiddleware (/workspace/.yarn/cache/@slack-bolt-npm-3.11.0-1183767d75-25491d656e.zip/node_modules/@slack/bolt/src/middleware/process.ts:23:47)
      at processMiddleware (/workspace/.yarn/cache/@slack-bolt-npm-3.11.0-1183767d75-25491d656e.zip/node_modules/@slack/bolt/src/middleware/process.ts:35:10)

Attachments:

Logs, screenshots, screencast, sample project, funny gif, etc.

[srajiang]

@M1kep - The error message you are seeing is expected. next is not supplied as args to your action listener callback function. You do not explicitly have to call next in this case for other listeners you register to execute.

There is a way to define a listener middleware. Here is thedocumentation for that and the way to use next within the context of listener middleware. It may be suitable for your use case, I don't have enough detail on what you're trying to do to know for sure!

[M1kep]

If the action listener callback function is not supplied next as an arg, I'd think the type definition for that call wouldn't provide it in the list of available args?

[srajiang]

That I'd need to look further into. Thanks for clarifying, I'll take a look at what's going on with the type definition there...

[filmaj]

I've tweaked the title and labels in this issue to have this issue focus on improving the types in this situation.

[filmaj]

I've taken a look at this issue a bit more. I would summarize the issue like this:

• Bolt has a concept of Global vs. Listener middleware.
• Global middleware is stored internally within App.ts on the middleware array.
• Listener middleware is stored internally within App.ts on the listeners array.
• When bolt-js processes an event, it:
  • Dispatches the event through the chain of global middleware, then
  • Finally dispatches the event through the chain of listener middleware
    • Current code explicitly calls out that the "last" listener in a listener middleware chain should not get a next() method (also does so here).

I think the easy fix here is to provide a no-op next() function on the last listener in the listener middleware chain. That should address the runtime exception reported in this issue. The simplest use case of registering a single listener to an event technically does not honour the listener API (because there is no next callback available, even though the types we provide state it should exist). That said, I also question why you as an app developer would call next() if you are registering a single listener to an event - this makes me wonder if there is another assumption / expectation here.

@M1kep what did you expect to happen when you called next() in your original code example here?

            // Log action_ids of all actions when in Debug mode
            bot.appInstance?.action(/.*/, async ({ action, logger, next }) => {
                if ('action_id' in action) {
                    logger.debug(`Action Hit(${botName}): ${action.action_id}`);
                }
                await next();
            });

Not questioning or judging, just trying to make sure I understand what a developer's expectations are. Thanks for any info!

I the mean time, I will try my hand at PR to capture the problem in a test and see if my proposed fix could work to address this.

[M1kep]

Hey @filmaj,

It's been a bit since I was working on this, and have been away from TS for a bit so am little rusty..

If I recall correctly, I think this was mostly a case of the type hints leading to some confusion. When I saw that the action middleware/listener accepted next(), I'd assumed it should be called.

Ideally, the type signature wouldn't provide next at all, but I'm not sure that's a realistic expectation for the type system. I think a no-op next() would completely solve the issue in a non-breaking manner, possibly with a dev time warning statement?