Merge PR #393: fix(whatsapp): initialize data variable and close log handle on error paths

Authored by FarukEst. Fixes #392.

1. Initialize data={} before health-check loop to prevent NameError when
   resp.json() raises after http_ready is set to True.
2. Extract _close_bridge_log() helper and call on all return False paths
   to prevent file descriptor leaks on failed connection attempts.
   Refactors disconnect() to reuse the same helper.

gateway/platforms/whatsapp.py

@@ -186,11 +186,13 @@ class WhatsAppAdapter(BasePlatformAdapter):
             # Phase 2: wait for WhatsApp status: connected (up to 15s more).
             import aiohttp
             http_ready = False
+            data = {}
             for attempt in range(15):
                 await asyncio.sleep(1)
                 if self._bridge_process.poll() is not None:
                     print(f"[{self.name}] Bridge process died (exit code {self._bridge_process.returncode})")
                     print(f"[{self.name}] Check log: {self._bridge_log}")
+                    self._close_bridge_log()
                     return False
                 try:
                     async with aiohttp.ClientSession() as session:
@@ -206,10 +208,11 @@ class WhatsAppAdapter(BasePlatformAdapter):
                                     break
                 except Exception:
                     continue
-            
+
             if not http_ready:
                 print(f"[{self.name}] Bridge HTTP server did not start in 15s")
                 print(f"[{self.name}] Check log: {self._bridge_log}")
+                self._close_bridge_log()
                 return False
             
             # Phase 2: HTTP is up but WhatsApp may still be connecting.
@@ -221,6 +224,7 @@ class WhatsAppAdapter(BasePlatformAdapter):
                     if self._bridge_process.poll() is not None:
                         print(f"[{self.name}] Bridge process died during connection")
                         print(f"[{self.name}] Check log: {self._bridge_log}")
+                        self._close_bridge_log()
                         return False
                     try:
                         async with aiohttp.ClientSession() as session:
@@ -251,8 +255,18 @@ class WhatsAppAdapter(BasePlatformAdapter):
             
         except Exception as e:
             logger.error("[%s] Failed to start bridge: %s", self.name, e, exc_info=True)
+            self._close_bridge_log()
             return False
     
+    def _close_bridge_log(self) -> None:
+        """Close the bridge log file handle if open."""
+        if self._bridge_log_fh:
+            try:
+                self._bridge_log_fh.close()
+            except Exception:
+                pass
+            self._bridge_log_fh = None
+
     async def disconnect(self) -> None:
         """Stop the WhatsApp bridge and clean up any orphaned processes."""
         if self._bridge_process:
@@ -289,12 +303,7 @@ class WhatsAppAdapter(BasePlatformAdapter):
         
         self._running = False
         self._bridge_process = None
-        if self._bridge_log_fh:
+        self._close_bridge_log()
-            try:
-                self._bridge_log_fh.close()
-            except Exception:
-                pass
-            self._bridge_log_fh = None
         print(f"[{self.name}] Disconnected")
     
     async def send(

tests/gateway/test_whatsapp_connect.py

@@ -0,0 +1,270 @@
+"""Tests for WhatsApp connect() error handling.
+
+Regression tests for two bugs in WhatsAppAdapter.connect():
+
+1. Uninitialized ``data`` variable: when ``resp.json()`` raised after the
+   health endpoint returned HTTP 200, ``http_ready`` was set to True but
+   ``data`` was never assigned.  The subsequent ``data.get("status")``
+   check raised ``NameError``.
+
+2. Bridge log file handle leaked on error paths: the file was opened before
+   the health-check loop but never closed when ``connect()`` returned False.
+   Repeated connection failures accumulated open file descriptors.
+"""
+
+import asyncio
+from pathlib import Path
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+
+from gateway.config import Platform
+
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+class _AsyncCM:
+    """Minimal async context manager returning a fixed value."""
+
+    def __init__(self, value):
+        self.value = value
+
+    async def __aenter__(self):
+        return self.value
+
+    async def __aexit__(self, *exc):
+        return False
+
+
+def _make_adapter():
+    """Create a WhatsAppAdapter with test attributes (bypass __init__)."""
+    from gateway.platforms.whatsapp import WhatsAppAdapter
+
+    adapter = WhatsAppAdapter.__new__(WhatsAppAdapter)
+    adapter.platform = Platform.WHATSAPP
+    adapter.config = MagicMock()
+    adapter._bridge_port = 19876
+    adapter._bridge_script = "/tmp/test-bridge.js"
+    adapter._session_path = Path("/tmp/test-wa-session")
+    adapter._bridge_log_fh = None
+    adapter._bridge_log = None
+    adapter._bridge_process = None
+    adapter._running = False
+    adapter._message_queue = asyncio.Queue()
+    return adapter
+
+
+def _mock_aiohttp(status=200, json_data=None, json_side_effect=None):
+    """Build a mock ``aiohttp.ClientSession`` returning a fixed response."""
+    mock_resp = MagicMock()
+    mock_resp.status = status
+    if json_side_effect:
+        mock_resp.json = AsyncMock(side_effect=json_side_effect)
+    else:
+        mock_resp.json = AsyncMock(return_value=json_data or {})
+
+    mock_session = MagicMock()
+    mock_session.get = MagicMock(return_value=_AsyncCM(mock_resp))
+
+    return MagicMock(return_value=_AsyncCM(mock_session))
+
+
+def _connect_patches(mock_proc, mock_fh, mock_client_cls=None):
+    """Return a dict of common patches needed to reach the health-check loop."""
+    patches = {
+        "gateway.platforms.whatsapp.check_whatsapp_requirements": True,
+        "gateway.platforms.whatsapp.asyncio.create_task": MagicMock(),
+    }
+    base = [
+        patch("gateway.platforms.whatsapp.check_whatsapp_requirements", return_value=True),
+        patch.object(Path, "exists", return_value=True),
+        patch.object(Path, "mkdir", return_value=None),
+        patch("subprocess.run", return_value=MagicMock(returncode=0)),
+        patch("subprocess.Popen", return_value=mock_proc),
+        patch("builtins.open", return_value=mock_fh),
+        patch("gateway.platforms.whatsapp.asyncio.sleep", new_callable=AsyncMock),
+        patch("gateway.platforms.whatsapp.asyncio.create_task"),
+    ]
+    if mock_client_cls is not None:
+        base.append(patch("aiohttp.ClientSession", mock_client_cls))
+    return base
+
+
+# ---------------------------------------------------------------------------
+# _close_bridge_log() unit tests
+# ---------------------------------------------------------------------------
+
+class TestCloseBridgeLog:
+    """Direct tests for the _close_bridge_log() helper method."""
+
+    @staticmethod
+    def _bare_adapter():
+        from gateway.platforms.whatsapp import WhatsAppAdapter
+        a = WhatsAppAdapter.__new__(WhatsAppAdapter)
+        a._bridge_log_fh = None
+        return a
+
+    def test_closes_open_handle(self):
+        adapter = self._bare_adapter()
+        mock_fh = MagicMock()
+        adapter._bridge_log_fh = mock_fh
+
+        adapter._close_bridge_log()
+
+        mock_fh.close.assert_called_once()
+        assert adapter._bridge_log_fh is None
+
+    def test_noop_when_no_handle(self):
+        adapter = self._bare_adapter()
+
+        adapter._close_bridge_log()  # must not raise
+
+        assert adapter._bridge_log_fh is None
+
+    def test_suppresses_close_exception(self):
+        adapter = self._bare_adapter()
+        mock_fh = MagicMock()
+        mock_fh.close.side_effect = OSError("already closed")
+        adapter._bridge_log_fh = mock_fh
+
+        adapter._close_bridge_log()  # must not raise
+
+        assert adapter._bridge_log_fh is None
+
+
+# ---------------------------------------------------------------------------
+# data variable initialization
+# ---------------------------------------------------------------------------
+
+class TestDataInitialized:
+    """Verify ``data = {}`` prevents NameError when resp.json() fails."""
+
+    @pytest.mark.asyncio
+    async def test_no_name_error_when_json_always_fails(self):
+        """HTTP 200 sets http_ready but json() always raises.
+
+        Without the fix, ``data`` was never assigned and the Phase 2 check
+        ``data.get("status")`` raised NameError.  With ``data = {}``, the
+        check evaluates to ``None != "connected"`` and Phase 2 runs normally.
+        """
+        adapter = _make_adapter()
+
+        mock_proc = MagicMock()
+        mock_proc.poll.return_value = None  # bridge stays alive
+
+        mock_client_cls = _mock_aiohttp(
+            status=200, json_side_effect=ValueError("bad json"),
+        )
+        mock_fh = MagicMock()
+
+        patches = _connect_patches(mock_proc, mock_fh, mock_client_cls)
+
+        with patches[0], patches[1], patches[2], patches[3], patches[4], \
+             patches[5], patches[6], patches[7], patches[8], \
+             patch.object(type(adapter), "_poll_messages", return_value=MagicMock()):
+            # Must NOT raise NameError
+            result = await adapter.connect()
+
+        # connect() returns True (warn-and-proceed path)
+        assert result is True
+        assert adapter._running is True
+
+
+# ---------------------------------------------------------------------------
+# File handle cleanup on error paths
+# ---------------------------------------------------------------------------
+
+class TestFileHandleClosedOnError:
+    """Verify the bridge log file handle is closed on every failure path."""
+
+    @pytest.mark.asyncio
+    async def test_closed_when_bridge_dies_phase1(self):
+        """Bridge process exits during Phase 1 health-check loop."""
+        adapter = _make_adapter()
+
+        mock_proc = MagicMock()
+        mock_proc.poll.return_value = 1  # dead immediately
+        mock_proc.returncode = 1
+
+        mock_fh = MagicMock()
+        patches = _connect_patches(mock_proc, mock_fh)
+
+        with patches[0], patches[1], patches[2], patches[3], patches[4], \
+             patches[5], patches[6], patches[7]:
+            result = await adapter.connect()
+
+        assert result is False
+        mock_fh.close.assert_called_once()
+        assert adapter._bridge_log_fh is None
+
+    @pytest.mark.asyncio
+    async def test_closed_when_http_not_ready(self):
+        """Health endpoint never returns 200 within 15 attempts."""
+        adapter = _make_adapter()
+
+        mock_proc = MagicMock()
+        mock_proc.poll.return_value = None  # bridge alive
+
+        mock_client_cls = _mock_aiohttp(status=503)
+        mock_fh = MagicMock()
+        patches = _connect_patches(mock_proc, mock_fh, mock_client_cls)
+
+        with patches[0], patches[1], patches[2], patches[3], patches[4], \
+             patches[5], patches[6], patches[7], patches[8]:
+            result = await adapter.connect()
+
+        assert result is False
+        mock_fh.close.assert_called_once()
+        assert adapter._bridge_log_fh is None
+
+    @pytest.mark.asyncio
+    async def test_closed_when_bridge_dies_phase2(self):
+        """Bridge alive during Phase 1 but dies during Phase 2."""
+        adapter = _make_adapter()
+
+        # Phase 1 (15 iterations): alive.  Phase 2 (iteration 16): dead.
+        call_count = [0]
+
+        def poll_side_effect():
+            call_count[0] += 1
+            return None if call_count[0] <= 15 else 1
+
+        mock_proc = MagicMock()
+        mock_proc.poll.side_effect = poll_side_effect
+        mock_proc.returncode = 1
+
+        # Health returns 200 with status != "connected" -> triggers Phase 2
+        mock_client_cls = _mock_aiohttp(
+            status=200, json_data={"status": "disconnected"},
+        )
+        mock_fh = MagicMock()
+        patches = _connect_patches(mock_proc, mock_fh, mock_client_cls)
+
+        with patches[0], patches[1], patches[2], patches[3], patches[4], \
+             patches[5], patches[6], patches[7], patches[8]:
+            result = await adapter.connect()
+
+        assert result is False
+        mock_fh.close.assert_called_once()
+        assert adapter._bridge_log_fh is None
+
+    @pytest.mark.asyncio
+    async def test_closed_on_unexpected_exception(self):
+        """Popen raises, outer except block must still close the handle."""
+        adapter = _make_adapter()
+
+        mock_fh = MagicMock()
+
+        with patch("gateway.platforms.whatsapp.check_whatsapp_requirements", return_value=True), \
+             patch.object(Path, "exists", return_value=True), \
+             patch.object(Path, "mkdir", return_value=None), \
+             patch("subprocess.run", return_value=MagicMock(returncode=0)), \
+             patch("subprocess.Popen", side_effect=OSError("spawn failed")), \
+             patch("builtins.open", return_value=mock_fh):
+            result = await adapter.connect()
+
+        assert result is False
+        mock_fh.close.assert_called_once()
+        assert adapter._bridge_log_fh is None