mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(slack): close clients on gateway shutdown
This commit is contained in:
parent
caf8e2f214
commit
45556b71ce
2 changed files with 89 additions and 0 deletions
|
|
@ -10,6 +10,7 @@ Uses slack-bolt (Python) with Socket Mode for:
|
|||
|
||||
import asyncio
|
||||
import contextvars
|
||||
import inspect
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
|
|
@ -704,6 +705,31 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
# first ping/pong as evidence of a wedged transport.
|
||||
self._socket_first_ping_grace_s = 60.0
|
||||
|
||||
async def _close_workspace_clients(self) -> None:
|
||||
"""Close any Slack SDK clients that may own aiohttp sessions."""
|
||||
clients: List[Any] = []
|
||||
if self._app is not None:
|
||||
primary_client = getattr(self._app, "client", None)
|
||||
if primary_client is not None:
|
||||
clients.append(primary_client)
|
||||
clients.extend(self._team_clients.values())
|
||||
|
||||
seen_ids: set[int] = set()
|
||||
for client in clients:
|
||||
ident = id(client)
|
||||
if ident in seen_ids:
|
||||
continue
|
||||
seen_ids.add(ident)
|
||||
|
||||
for method_name in ("close", "aclose"):
|
||||
closer = getattr(client, method_name, None)
|
||||
if not callable(closer):
|
||||
continue
|
||||
result = closer()
|
||||
if inspect.isawaitable(result):
|
||||
await result
|
||||
break
|
||||
|
||||
def _start_socket_mode_handler(self) -> None:
|
||||
"""Start the Slack Socket Mode background task."""
|
||||
if not self._app or not self._app_token:
|
||||
|
|
@ -1334,6 +1360,7 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
# receive every Slack event and dispatch it twice, producing double
|
||||
# responses — the same bug that affected DiscordAdapter (#18187).
|
||||
await self._stop_socket_mode_handler()
|
||||
await self._close_workspace_clients()
|
||||
self._app = None
|
||||
self._app_token = app_token
|
||||
self._proxy_url = proxy_url
|
||||
|
|
@ -1651,9 +1678,14 @@ class SlackAdapter(BasePlatformAdapter):
|
|||
)
|
||||
|
||||
await self._stop_socket_mode_handler()
|
||||
await self._close_workspace_clients()
|
||||
self._app = None
|
||||
self._app_token = None
|
||||
self._proxy_url = None
|
||||
self._bot_user_id = None
|
||||
self._team_clients = {}
|
||||
self._team_bot_user_ids = {}
|
||||
self._channel_team = {}
|
||||
|
||||
self._release_platform_lock()
|
||||
|
||||
|
|
|
|||
|
|
@ -468,6 +468,7 @@ class TestSlackConnectCleanup:
|
|||
)
|
||||
|
||||
second_handler = MagicMock()
|
||||
second_handler.close_async = AsyncMock(return_value=None)
|
||||
# _start_socket_mode_handler awaits the result of start_async via
|
||||
# asyncio.create_task — so the stub must return a real coroutine, not a
|
||||
# bare MagicMock.
|
||||
|
|
@ -490,6 +491,62 @@ class TestSlackConnectCleanup:
|
|||
first_handler.close_async.assert_awaited_once_with()
|
||||
assert adapter._handler is second_handler
|
||||
|
||||
with patch("gateway.status.release_scoped_lock"):
|
||||
await adapter.disconnect()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_disconnect_closes_workspace_clients_and_clears_runtime_state(self):
|
||||
"""Regression for #51465: shutdown must close Slack WebClients.
|
||||
|
||||
``hermes gateway run --replace`` takes the old process through the
|
||||
normal adapter.disconnect() path. If Slack leaves AsyncWebClient
|
||||
instances open there, aiohttp logs ``Unclosed client session`` while
|
||||
the old gateway exits after SIGTERM.
|
||||
"""
|
||||
config = PlatformConfig(enabled=True, token="xoxb-fake")
|
||||
adapter = SlackAdapter(config)
|
||||
|
||||
socket_task = asyncio.create_task(_pending_for_fake_task())
|
||||
handler = MagicMock()
|
||||
handler.close_async = AsyncMock(return_value=None)
|
||||
|
||||
primary_client = MagicMock()
|
||||
primary_client.close = AsyncMock(return_value=None)
|
||||
team_client = MagicMock()
|
||||
team_client.close = AsyncMock(return_value=None)
|
||||
|
||||
adapter._running = True
|
||||
adapter._handler = handler
|
||||
adapter._socket_mode_task = socket_task
|
||||
adapter._app = MagicMock()
|
||||
adapter._app.client = primary_client
|
||||
adapter._team_clients = {"T_FAKE": team_client}
|
||||
adapter._team_bot_user_ids = {"T_FAKE": "U_BOT"}
|
||||
adapter._channel_team = {"C_FAKE": "T_FAKE"}
|
||||
adapter._platform_lock_scope = "slack-app-token"
|
||||
adapter._platform_lock_identity = "xapp-fake"
|
||||
adapter._app_token = "xapp-fake"
|
||||
adapter._proxy_url = "http://proxy.example.com:3128"
|
||||
adapter._bot_user_id = "U_BOT"
|
||||
|
||||
with patch("gateway.status.release_scoped_lock") as mock_release:
|
||||
await adapter.disconnect()
|
||||
|
||||
handler.close_async.assert_awaited_once_with()
|
||||
primary_client.close.assert_awaited_once_with()
|
||||
team_client.close.assert_awaited_once_with()
|
||||
assert socket_task.cancelled()
|
||||
assert adapter._app is None
|
||||
assert adapter._handler is None
|
||||
assert adapter._socket_mode_task is None
|
||||
assert adapter._team_clients == {}
|
||||
assert adapter._team_bot_user_ids == {}
|
||||
assert adapter._channel_team == {}
|
||||
assert adapter._bot_user_id is None
|
||||
assert adapter._app_token is None
|
||||
assert adapter._proxy_url is None
|
||||
mock_release.assert_called_once_with("slack-app-token", "xapp-fake")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# TestSlackSocketWatchdog
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue