Socket Mode Rewrite to Python(ish) Implementation

[filmaj]

Socket Mode Rewrite

This PR ports, as much as possible, the Python implementation of the Socket Mode client to node.

I have been running a longevity test of an app, in production, on a busy workspace, using this PR since April 24th without issue (the app reconnects gracefully when Slack sends a disconnect message, which is what we expect).

Why?

The existing node socket-mode package uses finity, an old finite state machine configuration package. While it has served us well, it has been difficult to maintain, with tricky semantics. Additionally, the existing socket-mode implementation is almost a straight copy-paste of the rtm-api module, which is very old.

Breaking Changes

• Removed two lifecycle events - though we can add them back in if we think they are useful / worth keeping for backwards compatibility:
  • authenticated. I did not find it useful, it just represented that the client retrieved the WS URL to connect to via a successful response from the apps.connections.open HTTP API.
  • unable_to_start_socket_mode. I felt like the error event was sufficient?
• Some properties and methods that were previously public on SocketModeClient have been removed: connected, authenticated and isActive(), though isActive() could easily be added back in.

What's The Same?

Many of the developer-facing APIs have remained the same:

• the EventEmitter-based design, where the client emits the events described in the README, remain identical. This should ease the upgrading path for any projects dependent on this package, including bolt-js.
• I think SocketModeClient's standard public API also remains the same? start() and disconnect() for sure, but isActive() is now moved to SlackWebSocket, so that's a major change - though to be fair, we never documented these APIs.

Internal Design Changes

• SocketModeClient aims to implement similar interfaces as Python's SocketModeClient class, as well as its BaseSocketModeClient.
  • The Client is responsible for retrieving the WebSocket URL to connect to (by interacting with the apps.connections.open HTTP API) and managing reconnections. We can consider these as application-level concerns.
• SlackWebSocket aims to implement similar interfaces as Python's Connection class. Luckily for the node implementation, many of the low-level details of the WebSocket protocol are encapsulated in the ws dependency.
  • SlackWebSocket concerns itself with managing and maintaining a single WebSocket connection and nothing more; any WS-specific events relevant to application functioning (e.g. disconnects) are raised back to the Client for it to deal with.

IMO this leads to a nice separation of concerns.

Concerns / Questions / Request for Feedback

• Moving to an object-oriented / class-based setup, actually unit testing the private methods in JavaScript is.. impossible? So I think the code coverage from unit tests will drop significantly as a result. Any suggestions on how to tackle this? Could just make methods public and prefix with an _ just to expose them for unit tests?
• Seems like the documentation on https://slack.dev/node-slack-sdk/socket-mode is just a copy paste of the README (and even the comments in the readme markdown state as much). Can we instead link to the README from the slack.dev site? I feel like the chances of the package documentation being kept up to date are better by being in the README.

[filmaj]

Thanks for the reviews! I did see some CI failures with the integration tests, and I realized some of the sequencing of events / websocket behaviour in the test was not ideal and probably brittle on a weaker machines (like GitHub Actions). Running them once on my local machine tended to pass them, but if I ran the tests like this instead:

while npm run test:integration; do echo "OK"; done

.. after about 10 or so runs I could trigger a failure.

I made a change in the tests to make them more resilient (any event-based waiting promises should be created after start() completes, not after any timeouts; ensures the event loop has opportunity to execute a few iterations to ensure events get picked up properly by any waiters) and now they pass on both local and CI at a much higher rate.

Also confirming that the longevity test I had set up has been operating flawlessly for a week.

Merging this! Hooray! I will likely publish a 2.0 release of socket-mode soon!