RFC: Slapp v4.0 Proposal

[shaydewael]

Background

As a result of the Missions acquisition, two of Slapp's two core contributors began working on other projects. After sitting down with @mbrevoort and @selfcontained, we determined that it’s worthwhile for the developer community to invest Slack’s resources into supporting and maintaining the Slapp project and community.

We started by triaging the open issues on the Slapp repository and looking at changes (small and large) that we saw important to address in the next major version of the framework. We came up with a few different focus areas where we see important, outstanding work to be addressed:

Core framework

Goals

1. Enable more granular, event-based middleware
2. Develop a standardized API interface for Slapp receivers

Proposed changes

• Changing the parameters a listener function has access to by putting them in an object. This allows an arbitrary number of listener middleware functions (detailed below) on an event. These parameters can be destructured so listener functions only have to pick the ones they need access to:
  • payload: object (aliases message, event) - The contents of the event. The exact structure will depend on which type of event this listener is attached to.
  • context: object - The event context. This object contains data about the message and the app.
  • say?: (string|object) - A function to respond to an incoming event. Only available when the listener is triggered for an event that contains a channel_id. Accepts simple strings and message objects.
  • ack?: (string|object|undefined) - A function to acknowledge that an incoming event was received by the app. This is detailed in more detail in a comment below on this issue.
  • respond?: (string|object) - A function to respond to an incoming event. It's only available when the listener is triggered for an event which contains a response_url.
  • body: object - An object that contains the whole body of the event, which is a superset of the data inside of payload. It's rarely needed, but contains data like api_app_id and authed_users.

A listener function may now look something like this:

// Slash command example responding
slapp.command('/inorout', ({event, respond}) => {
  respond('<@' + event.user_id + '> triggered a slash command');
});

• Add support to attach an arbitrary number of listener middleware functions to handlers. This will prevent developers from having to process and redirect events/subevents. This could be done by allowing optional middleware functions to be passed to an event:

// A sample middleware function
function noBotMessages({message, next}) {
  if (!message.subtype || message.subtype !== ‘bot_message’) {
    next();
  }
}

slapp.message(noBotMessages, ({message}) => {
  //...
})

• Add the functionality to filter events by subtype (proposed in filter event by type and subtype #37). Right now, developers can only sort by event, and must use their own logic to filter that event further. While @selfcontained's proposed solution makes sense, it isn't intuitive for one of the most common use cases: filter for events with no subtype. With that in mind, one solution is offering a default listener middleware that can filter by subtype:

// this is the current way
slapp.message('example', ({message, say}) => {
  //filter only subtype of bot_message using your own logic
})

// this would be using the new listener middleware
slapp.message(subtype(‘bot_message’), ‘example’, ({message, say}) => {
  // this would handle only message events with `bot_message` subtype
})

• Create an abstracted receiver interface. Related to #83. A proposed interface that should work with Express, serverless, and WebSocket environments:

Receiver {
     constructor(opts) {
          super()
         // opts no longer needs to contain context -- that will be handled in the Slapp handler
     }

     on('message', listener: (payload: any)  => void) {
          // Registers a listener for incoming events that this receiver wants Slapp to process
          // This method is called by Slapp, and users should not need to call it directly
     }

     on('error', errorHandler: (error: Error)  => void) {
          // Registers a listener for errors that this receiver wants Slapp to process
          // This method is called by Slapp, and users should not need to call it directly
     }

     respond(error?: Error, payload: object) {
          // This will handle responding to Slack (using a response URL or even just a 200 OK message).
          // Open question: What happens if a developer doesn't respond? Do we let the error throw?
     }
}

There can be many implementations of the Receiver interface to serve different use cases. For example, the existing receiver in the project would become an ExpressReceiver - and it would still be the default when initializing Slapp. Other Receiver implementations (e.g. a LambdaReceiver, MockReceiver, WebSocketReceiver, etc) will dictate how they need to be initialized, including a different set of options.

• The changes above imply that Receivers no longer have to deal with the context object. The receiver simply delivers raw event payloads (whose shape is yet to be defined) to Slapp. Then Slapp will apply the context to build the object(s) needed when routing that payload through middleware and handlers.

Developer experience

Goals

1. Make it simple (and hopefully pleasant) to get started using the Slapp framework
2. Provide adequate documentation and example code for new and existing developers
3. Make Slapp work well with other Slack developer tooling

Proposed changes

• Rewrite the documentation, including a Getting Started guide to make it simple to get up and running. There are also examples that no longer work on the README, so make sure all examples work and are up-to-date. (Fixes Demos and README don't work #113 and Getting Started / Slapp Example #114)

• Change the client in the framework to the official Node Slack client. Originally, the official Slack client wasn't chosen because it was heavyweight and didn't add much benefit. A few years have passed, and we think the reduction in package size and features like rate limit and retry handling, pagination support, and refresh token support, the official client is in a better place to support most use cases for Slapp.

• The logging in Slapp is useful, but is less informative than we think it could be. In some of our other Node packages, we use four labels (ERROR, WARN, INFO, and DEBUG) to classify logs for developers. This comes along with more granular control over logging. It's likely that we'd modify the existing logger to work closely to the node-slack-sdk implementation. (Explained more in Add more descriptive logging #118)

[shaydewael]

I have a couple of open questions that I want to work on developing further as well:

• What should the shape of msg look like? Right now, the receiver injects the Slack payload with extra information that's important to the developer. I think it's necessary to keep this information somewhere, but maybe it's worth exploring how we can more cleanly separate this. Maybe we keep the raw Slack payload in msg.payload and move auxiliary information (like usersMentioned) outside of the raw payload to msg.usersMentioned.
• Should the shape of middleware signatures stay the same? If we decouple the receiver from express, it may be valuable to look at how we're handling middleware. Right now, I'm leaning towards keeping it the same: (msg: object, next: function), but maybe it's worth considering alternatives like (msg: object, next: function, done: function) which would give developers more control over the execution chain of the Slapp middleware.
• Are there easy ways to make it simple to create and publish community scripts? This probably isn't in scope for V1, but how can we design the framework so that in the future, there are common community scripts that are easy to reuse in individual developer's projects?

[simonsayscode]

The various proposed changes for both core framework and developer experience are great. Especially like the generalized receiver to make it easier for attaching to other non-Express architectures as well as the migration to Slack's library for api calls.

Msg shape: agree that having a cleaner separation of raw to library added would make sense - especially if with a proposed new middleware section, that would create a standardized place for injected data to live.

Re: middleware signatures, I like the (msg: object, next: function) as it resembles express middleware signatures. Haven't seen the addition of a done: function before. Is there a pattern or other library that that's common so I can see how it's been used?

[shaydewael]

Some considerations came up when thinking through the middleware design...

Acknowledging events

To ensure a consistent user experience in the Slack client, some types of events need to be acknowledged (generally within 3 seconds). This includes all actions, commands, and options requests.

A possible way to handle this is by passing an ack() function into these events. What an app passes to ack() would be different depending on the kind of event:

• Button clicks, menu selections, and slash commands: either call ack() with no parameters or a string/object to update the message.
• Actions: call ack() with no parameters
• Dialog submissions: call ack() with no parameters or with an an object describing validation errors
• Options requests: call ack() with an object containing options for user to see

An app would need to call ack() within the time limit or Slapp would generate an error.

[simonsayscode]

Is there a default 200 response/ack already sent in these timeouts? https://github.com/MissionsAI/slapp/blob/a47628a0df9a92013629509c14446544bc7975d0/src/message.js#L109

Various event type parsing all set their response timeout in similar fashions: https://github.com/MissionsAI/slapp/blob/aa0217f9b6763ffbb4c35ac39fc230c92ab7679d/src/receiver/middleware/parse-command.js#L25

If it doesn't necessarily send back a response, I believe it then defaults to using the response_url for future responding to messages and it doesn't seem to affect functionality (other than ack'ing a dialog). Or are you proposing logging an informative error regardless just to ensure the best practice of speedy response time and to identify bottlenecks/slow areas?

[shaydewael]

@simonsayscode Including this kind of response timeout causes some tricky edge cases. For example, what if your network latency causes your app to receive requests in 600ms? In that case, you'd only have 2400ms to respond which will cause an error (and no way to handle the error). The other case is if the response timeout beats the developer to sending a response, and then the developer's correct response is lost.

While we may be able to default back to using the response_url in some cases, for dialog submissions and options requests, it's more of an issue.

Overall, it's just much more deterministic and leads to better UX to ask all developers to call ack() (or some other function). I'm interested to see what others developers think of it. If it's something that seems important, we could potentially introduce a configuration option that allows you to opt-in (not default) to sending ack(), although I think we should still try to discourage this.