hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases Wiki Activity

fix(whatsapp): install a missing aiohttp through PM and name every failed connect

Browse Source 
A gateway whose dependency environment lacks aiohttp (a sealed env from
`hermes update` with a partial messaging extra, #126358) spawned the bridge,
then every /health poll raised ModuleNotFoundError inside
`_poll_bridge_health`'s blanket except. connect() returned False with no
fatal code, so the reconnect watcher retried forever while the bridge
answered /health 200, and telemetry counted each attempt as error_class
"other".

- connect() now obtains aiohttp through PM (`ensure_import("sms")`, the
  extra that is exactly aiohttp) before starting the bridge, the same
  lazy-install path Slack/Teams use. When PM cannot install it (lazy
  installs disabled, foreign interpreter, install failure) or aiohttp is a
  half-removed namespace shell (#71308), it fails with the named,
  non-retryable `whatsapp_aiohttp_missing` and never spawns the bridge.
- The bridge-died / "did not start in 15s" paths set a retryable fatal code
  (`whatsapp_bridge_exited` / `whatsapp_bridge_timeout`), so reconnect
  status and connect telemetry name the cause instead of "other".
- Folds the salvaged test file into test_whatsapp_connect.py as two
  invariant tests.
This commit is contained in:
Teknium
2026-10-04 18:35:28 -07:00
parent f5e21913cc
commit 2dda207409
3 changed files with 72 additions and 124 deletions
Download Patch File Download Diff File Expand all files Collapse all files

52
plugins/platforms/whatsapp/adapter.py
View File

@@ -264,12 +264,33 @@ def check_whatsapp_requirements() -> bool:
return False
def _aiohttp_available() -> bool:
def _import_aiohttp():
import aiohttp
if not hasattr(aiohttp, "ClientSession"): # half-removed install leaves a namespace shell (#71308)
raise ImportError(f"aiohttp at {list(aiohttp.__path__)} is an incomplete install")
return aiohttp
def ensure_aiohttp() -> Optional[str]:
"""None once aiohttp imports (PM installs the ``sms`` extra, which is exactly aiohttp, when it is missing), else why not.
Every bridge call imports aiohttp; a sealed env without it used to fail each /health poll silently and loop
forever on "did not start in 15s" while the bridge was healthy (#126358).
"""
try:
import aiohttp # noqa: F401
return True
except ImportError:
return False
_import_aiohttp()
return None
except ImportError as exc:
if not isinstance(exc, ModuleNotFoundError):
return str(exc) # present but broken: PM sees it as installed, reinstalling is the user's call
try:
from pm.extras import ensure_import
ensure_import("sms")
_import_aiohttp()
return None
except Exception as exc:
return str(exc) or type(exc).__name__
# Env vars bridge.js consumes; injected because a multiplexed subprocess's os.environ lacks the secondary profile's .env.
@@ -489,10 +510,12 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter):
bridge_env.update(HERMES_IMAGE_CACHE_DIR=str(img_dir), HERMES_AUDIO_CACHE_DIR=str(audio_dir), HERMES_DOCUMENT_CACHE_DIR=str(doc_dir))
return bridge_env
def _bridge_died(self, detail: str) -> bool:
def _bridge_died(self, detail: str, code: str = "whatsapp_bridge_exited") -> bool:
print(f"[{self.name}] {detail}")
print(f"[{self.name}] Check log: {self._bridge_log}")
self._close_bridge_log()
# Named so the reconnect status and connect telemetry say why instead of an unclassified failure.
self._set_fatal_error(code, f"{detail} (see {self._bridge_log})", retryable=True)
return False
async def _poll_bridge_health(self, died_msg: str) -> tuple[Optional[bool], bool, dict]:
@@ -522,7 +545,7 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter):
if connected is False:
return False
if not http_ready:
return self._bridge_died("Bridge HTTP server did not start in 15s")
return self._bridge_died("Bridge HTTP server did not start in 15s", "whatsapp_bridge_timeout")
if data.get("status") != "connected":
print(f"[{self.name}] Bridge HTTP ready, waiting for WhatsApp connection...")
connected, _, _ = await self._poll_bridge_health("Bridge process died during connection")
@@ -535,18 +558,12 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter):
return True
def _preflight(self) -> bool:
"""Node + aiohttp + bridge script + creds.json present, else a non-retryable fatal error (an unpaired bridge only prints QR codes; retries would pay 30s each)."""
"""Node + bridge script + creds.json present, else a non-retryable fatal error (an unpaired bridge only prints QR codes; retries would pay 30s each)."""
bridge_path = Path(self._bridge_script)
creds_path = self._session_path / "creds.json"
checks = (
(check_whatsapp_requirements, ("[%s] Node.js not found. WhatsApp requires Node.js.", self.name),
"whatsapp_node_missing", "Node.js is not installed — install Node.js and re-run `hermes gateway`."),
# Every bridge health poll imports aiohttp; a sealed env that shipped a partial messaging extra
# used to swallow the ModuleNotFoundError and loop forever on "did not start in 15s" (#126358).
(_aiohttp_available, ("[%s] aiohttp not installed — the WhatsApp bridge health probe needs it.", self.name),
"whatsapp_aiohttp_missing",
"aiohttp is not installed — the WhatsApp bridge health probe needs it. "
"Run `hermes update` (or `uv pip install \"aiohttp==3.14.3\"`), then restart `hermes gateway`."),
(bridge_path.exists, ("[%s] Bridge script not found: %s", self.name, bridge_path),
"whatsapp_bridge_missing", f"WhatsApp bridge script missing at {bridge_path}."),
(creds_path.exists, ("[%s] WhatsApp is enabled but not paired (no creds.json at %s). Pair from the dashboard or run "
@@ -586,6 +603,13 @@ class WhatsAppAdapter(WhatsAppBehaviorMixin, BasePlatformAdapter):
return False
if not self._preflight():
return False
if (why := await asyncio.to_thread(ensure_aiohttp)) is not None:
message = (f"aiohttp is unavailable ({why}); the WhatsApp bridge client needs it. "
"Run `hermes pm install --extra sms` (it ships aiohttp; `hermes pm repair` for a damaged install), "
"then restart `hermes gateway`.")
logger.warning("[%s] %s", self.name, message)
self._set_fatal_error("whatsapp_aiohttp_missing", message, retryable=False)
return False
bridge_path = Path(self._bridge_script)
lock_acquired = False
try:

110
tests/gateway/test_whatsapp_aiohttp_missing.py
View File

@@ -1,110 +0,0 @@
"""WhatsApp must name a missing aiohttp instead of looping forever (#126358).
A sealed env built by ``hermes update`` can ship a partial messaging extra
without aiohttp. The adapter's requirement check only verified Node.js, so
``connect()`` spawned the bridge and every health poll raised
``ModuleNotFoundError`` — swallowed by ``except Exception: continue`` — and
the gateway looped forever on the misleading
"Bridge HTTP server did not start in 15s".
"""
import asyncio
import sys
from pathlib import Path
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from gateway.config import Platform
def _make_adapter():
"""Create a WhatsAppAdapter with test attributes (bypass __init__)."""
from plugins.platforms.whatsapp.adapter import WhatsAppAdapter
adapter = WhatsAppAdapter.__new__(WhatsAppAdapter)
adapter.platform = Platform.WHATSAPP
adapter.config = MagicMock()
adapter._bridge_port = 19877
adapter._bridge_script = "/tmp/test-bridge-126358.js"
adapter._session_path = Path("/tmp/test-wa-session-126358")
adapter._bridge_log_fh = None
adapter._bridge_log = None
adapter._bridge_process = None
adapter._reply_prefix = None
adapter._send_read_receipts = False
adapter._dm_policy = adapter._group_policy = "pairing"
adapter._allow_from = adapter._group_allow_from = set()
adapter._running = False
adapter._message_handler = None
adapter._fatal_error_code = None
adapter._fatal_error_message = None
adapter._fatal_error_retryable = True
adapter._fatal_error_handler = None
adapter._active_sessions = {}
adapter._pending_messages = {}
adapter._background_tasks = set()
adapter._auto_tts_disabled_chats = set()
adapter._message_queue = asyncio.Queue()
adapter._http_session = None
return adapter
class TestMissingAiohttpNamed:
@pytest.mark.asyncio
async def test_connect_fails_fast_with_named_aiohttp_error(self):
"""Node present + aiohttp missing -> non-retryable fatal naming aiohttp, no bridge spawn."""
from plugins.platforms.whatsapp.adapter import WhatsAppAdapter
adapter = _make_adapter()
with patch(
"plugins.platforms.whatsapp.adapter.find_node_executable",
return_value="/fake/node",
), patch.object(Path, "exists", return_value=True), patch(
"subprocess.run", return_value=MagicMock(returncode=0)
), patch(
"subprocess.Popen", return_value=MagicMock()
) as mock_popen, patch(
"plugins.platforms.whatsapp.adapter.asyncio.sleep",
new_callable=AsyncMock,
), patch.dict(
sys.modules, {"aiohttp": None}
):
result = await adapter.connect()
assert result is False
# Fail fast: the bridge process must never be spawned.
mock_popen.assert_not_called()
# Named error, not the misleading "did not start in 15s" loop.
assert adapter.fatal_error_code == "whatsapp_aiohttp_missing"
assert "aiohttp" in (adapter.fatal_error_message or "").lower()
assert adapter.fatal_error_retryable is False
def test_preflight_names_missing_aiohttp(self):
"""_preflight alone reports whatsapp_aiohttp_missing when the import fails."""
adapter = _make_adapter()
with patch(
"plugins.platforms.whatsapp.adapter.check_whatsapp_requirements",
return_value=True,
), patch.object(Path, "exists", return_value=True), patch.dict(
sys.modules, {"aiohttp": None}
):
ok = adapter._preflight()
assert ok is False
assert adapter.fatal_error_code == "whatsapp_aiohttp_missing"
def test_preflight_passes_with_aiohttp(self):
"""With aiohttp importable the preflight keeps its Node/bridge/creds semantics."""
adapter = _make_adapter()
with patch(
"plugins.platforms.whatsapp.adapter.check_whatsapp_requirements",
return_value=True,
), patch.object(Path, "exists", return_value=True):
ok = adapter._preflight()
assert ok is True
assert adapter.fatal_error_code is None

34
tests/gateway/test_whatsapp_connect.py
View File

@@ -14,6 +14,7 @@ Regression tests for two bugs in WhatsAppAdapter.connect():
import asyncio
import signal
import sys
from pathlib import Path
from unittest.mock import AsyncMock, MagicMock, patch
@@ -544,3 +545,36 @@ class TestNoCredsPreflight:
# but the fatal-error code is NOT the "not paired" one.
assert result is False
assert adapter._fatal_error_code != "whatsapp_not_paired"
class TestNamedConnectFailures:
"""A failed connect names its cause; an unnamed False loops forever as an unclassified failure (#126358)."""
@pytest.mark.asyncio
async def test_missing_aiohttp_pm_cannot_install_is_named_and_final(self):
from pm.package import InstallError
adapter = _make_adapter()
adapter._acquire_platform_lock = MagicMock(return_value=False)
ensure_import = MagicMock(side_effect=InstallError("venv", "lazy installs are disabled"))
with patch("plugins.platforms.whatsapp.adapter.check_whatsapp_requirements", return_value=True), \
patch.object(Path, "exists", return_value=True), patch.dict(sys.modules, {"aiohttp": None}), \
patch("pm.extras.ensure_import", ensure_import):
assert await adapter.connect() is False
ensure_import.assert_called_once_with("sms")
adapter._acquire_platform_lock.assert_not_called() # bridge never started
assert adapter.fatal_error_code == "whatsapp_aiohttp_missing"
assert adapter.fatal_error_retryable is False
assert "lazy installs are disabled" in adapter.fatal_error_message
@pytest.mark.asyncio
async def test_bridge_http_timeout_is_a_named_retryable_failure(self):
adapter = _make_adapter()
adapter._bridge_process = MagicMock(**{"poll.return_value": None})
with patch.object(adapter, "_probe_bridge_health", new_callable=AsyncMock, side_effect=OSError("refused")), \
patch("plugins.platforms.whatsapp.adapter.asyncio.sleep", new_callable=AsyncMock):
assert await adapter._wait_for_bridge() is False
assert adapter.fatal_error_code == "whatsapp_bridge_timeout"
assert adapter.fatal_error_retryable is True