mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-08 03:01:47 +00:00
fix(slack): close previous handler in connect() to prevent zombie Socket Mode connections
SlackAdapter.connect() overwrote self._handler, self._app, and self._socket_mode_task without closing the prior AsyncSocketModeHandler first. If connect() was called a second time on the same adapter (e.g. during a gateway restart or in-process reconnect attempt), the old Socket Mode websocket stayed alive. Both the old and new connections received every Slack event and dispatched it twice — producing double responses with different wording, the same bug that affected DiscordAdapter (#18187, fixed in #18758). Fix: add a close-before-reassign guard at the start of the connection setup path, mirroring the guard DiscordAdapter.connect() already has. When self._handler is None (fresh adapter, first connect()) the block is a harmless no-op. Scoped to the handler/app fields only — no behavior change for any path that does not call connect() twice. Fixes #18980
This commit is contained in:
parent
c14bf441a3
commit
6c1322b997
2 changed files with 64 additions and 0 deletions
|
|
@ -528,6 +528,21 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
return False
|
||||
lock_acquired = True
|
||||
|
||||
# Close any previous handler before creating a new one so that
|
||||
# calling connect() a second time (e.g. during a gateway restart or
|
||||
# in-process reconnect attempt) does not leave a zombie Socket Mode
|
||||
# connection alive. Both the old and new connections would otherwise
|
||||
# receive every Slack event and dispatch it twice, producing double
|
||||
# responses — the same bug that affected DiscordAdapter (#18187).
|
||||
if self._handler is not None:
|
||||
try:
|
||||
await self._handler.close_async()
|
||||
except Exception:
|
||||
logger.debug("[%s] Failed to close previous Slack handler", self.name)
|
||||
finally:
|
||||
self._handler = None
|
||||
self._app = None
|
||||
|
||||
# First token is the primary — used for AsyncApp / Socket Mode
|
||||
primary_token = bot_tokens[0]
|
||||
self._app = AsyncApp(token=primary_token)
|
||||
|
|
|
|||
|
|
@ -231,6 +231,55 @@ class TestSlackConnectCleanup:
|
|||
mock_release.assert_called_once_with("slack-app-token", "xapp-fake")
|
||||
assert adapter._platform_lock_identity is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reconnect_closes_previous_handler_to_prevent_zombie_socket(self):
|
||||
"""Regression for #18980: calling connect() on an adapter that already has
|
||||
a live handler (e.g. during a gateway restart) must close the old
|
||||
AsyncSocketModeHandler before creating a new one. Without this guard,
|
||||
the old Socket Mode websocket stays alive and both connections dispatch
|
||||
every Slack event, producing double responses — the same bug that
|
||||
affected DiscordAdapter (#18187).
|
||||
"""
|
||||
config = PlatformConfig(enabled=True, token="xoxb-fake")
|
||||
adapter = SlackAdapter(config)
|
||||
|
||||
# Simulate state left over from a prior connect() call.
|
||||
first_handler = AsyncMock()
|
||||
first_handler.close_async = AsyncMock()
|
||||
adapter._handler = first_handler
|
||||
|
||||
mock_app = MagicMock()
|
||||
def _noop_decorator(event_type):
|
||||
def decorator(fn): return fn
|
||||
return decorator
|
||||
mock_app.event = _noop_decorator
|
||||
mock_app.command = _noop_decorator
|
||||
mock_app.action = _noop_decorator
|
||||
mock_app.client = AsyncMock()
|
||||
|
||||
mock_web_client = AsyncMock()
|
||||
mock_web_client.auth_test = AsyncMock(return_value={
|
||||
"user_id": "U_BOT",
|
||||
"user": "testbot",
|
||||
"team_id": "T_FAKE",
|
||||
"team": "FakeTeam",
|
||||
})
|
||||
|
||||
second_handler = MagicMock()
|
||||
|
||||
with patch.object(_slack_mod, "AsyncApp", return_value=mock_app), \
|
||||
patch.object(_slack_mod, "AsyncWebClient", return_value=mock_web_client), \
|
||||
patch.object(_slack_mod, "AsyncSocketModeHandler", return_value=second_handler), \
|
||||
patch.dict(os.environ, {"SLACK_APP_TOKEN": "xapp-fake"}), \
|
||||
patch("gateway.status.acquire_scoped_lock", return_value=(True, None)), \
|
||||
patch("gateway.status.release_scoped_lock"), \
|
||||
patch("asyncio.create_task"):
|
||||
result = await adapter.connect()
|
||||
|
||||
assert result is True
|
||||
first_handler.close_async.assert_awaited_once_with()
|
||||
assert adapter._handler is second_handler
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestSlackProxyBehavior
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue