Summary

This pull request fixes #1065 by improving the internals of AsyncSocketModeClient.

As described in the bug report, the reconnection occasionally may fail to properly replace the active WebSocket connection in the connect_to_new_endpoint method.

If both the connection monitoring code and other code try to reconnect at the same time, a race condition issue can arise. In this case, the internal state self.wss_uri and self.current_session can be invalid (=stale). The invalid state is automatically repaired after a certain amount of time (say, less than 1 minute with the default setting) but the app can be unresponsive to any incoming requests until the recovery.

It's not so easy to reproduce the situation in unit tests but you can easily reproduce the situation by running the following script:

import logging
logging.basicConfig(level=logging.DEBUG)

import asyncio
import os
# pip install slack-sdk
from slack_sdk.socket_mode.response import SocketModeResponse
from slack_sdk.socket_mode.request import SocketModeRequest
# pip install "aiohttp>=3,<4"
from slack_sdk.web.async_client import AsyncWebClient
from slack_sdk.socket_mode.aiohttp import SocketModeClient

async def main():
    client = SocketModeClient(
        app_token=os.environ.get("SLACK_APP_TOKEN"),
        web_client=AsyncWebClient(token=os.environ.get("SLACK_BOT_TOKEN")),
    )
    async def process(client: SocketModeClient, req: SocketModeRequest):
        if req.type == "events_api":
            response = SocketModeResponse(envelope_id=req.envelope_id)
            await client.send_socket_mode_response(response)
            if req.payload["event"]["type"] == "message":
                await client.web_client.reactions_add(
                    name="eyes",
                    channel=req.payload["event"]["channel"],
                    timestamp=req.payload["event"]["ts"],
                )
    client.socket_mode_request_listeners.append(process)

    for _ in range(5):
        # concurrently reconnecting to new endpoints
        asyncio.ensure_future(client.connect_to_new_endpoint())
    # just for keeping this process running
    await asyncio.sleep(float("inf"))

asyncio.run(main())

To resolve this, we can do the same with the sync version of SocketModeClient. It always acquires a lock when replacing wss_uri and current_session to make the operation safe. The synchronization affects only the reconnection. The overall performance of the WeSocket message handlers is never affected.

If I remember correctly, there was no strong reason why I didn't have the lock in the async Socket Mode clients. I'm thinking that I just missed adding it during the initial development #883

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

• slack_sdk.web.WebClient (sync/async) (Web API client)
• slack_sdk.webhook.WebhookClient (sync/async) (Incoming Webhook, response_url sender)
• slack_sdk.socket_mode (Socket Mode client)
• slack_sdk.signature (Request Signature Verifier)
• slack_sdk.oauth (OAuth Flow Utilities)
• slack_sdk.models (UI component builders)
• slack_sdk.scim (SCIM API client)
• slack_sdk.audit_logs (Audit Logs API client)
• slack_sdk.rtm_v2 (RTM client)
• /docs-src (Documents, have you run ./docs.sh?)
• /docs-src-v2 (Documents, have you run ./docs-v2.sh?)
• /tutorial (PythOnBoardingBot tutorial)
• tests/integration_tests (Automated tests for this library)

Requirements (place an x in each [ ])

• 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 run python3 -m venv .venv && source .venv/bin/activate && ./scripts/run_validation.sh after making the changes.