Narrow the suppression in _socket_looks_dead

The helper caught bare Exception around both `transport.is_closing()`
and `reader.at_eof()`, which silently masked programmer bugs like a
misnamed attribute or an unexpected type — the helper always reported
"probably alive" and ROLLBACK proceeded against a broken protocol.

Narrow both clauses to (AttributeError, RuntimeError): the legitimate
failure modes for a mocked / partially torn-down transport, matching
the `_cleanup_loop_thread` precedent in the dbapi package. Anything
else now propagates so the underlying bug surfaces to the operator.

Add a regression test for each branch of the helper, including the
contract-change check: a `ValueError` from a broken mock now escapes
instead of being swallowed as "probably alive".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: antoineleclair

src/dqliteclient/pool.py

@@ -29,13 +29,18 @@ def _socket_looks_dead(conn: DqliteConnection) -> bool:
     writer = getattr(protocol, "_writer", None)
     reader = getattr(protocol, "_reader", None)
     transport = getattr(writer, "transport", None) if writer is not None else None
+    # Narrow suppression to the specific categories a mock / partially
+    # torn-down transport can legitimately raise. Wider `except Exception`
+    # would mask programmer bugs (e.g. a misnamed attribute introduced by a
+    # refactor). Mirrors the precedent set by ``_cleanup_loop_thread`` in
+    # ``dqlitedbapi.connection``.
     try:
         closing = transport.is_closing() if transport is not None else False
-    except Exception:
+    except (AttributeError, RuntimeError):
         closing = False
     try:
         eof = reader.at_eof() if reader is not None else False
-    except Exception:
+    except (AttributeError, RuntimeError):
         eof = False
     return (isinstance(closing, bool) and closing) or (isinstance(eof, bool) and eof)
 

tests/test_pool.py

@@ -1156,3 +1156,90 @@ async def test_fetchval_through_pool(self, mock_connection: MagicMock) -> None:
             assert result == 42
 
         await pool.close()
+
+
+class TestSocketLooksDead:
+    """Contract tests for the pool's ``_socket_looks_dead`` heuristic.
+
+    The helper must tolerate mocked / missing transport attributes (returning
+    ``False`` so ROLLBACK still gets attempted) but must NOT silently swallow
+    arbitrary exceptions — an unexpected error is a programmer bug that should
+    surface, not a signal of a dead socket.
+    """
+
+    def _make_conn(
+        self,
+        *,
+        transport: object | None = None,
+        reader: object | None = None,
+    ) -> MagicMock:
+        writer = MagicMock()
+        writer.transport = transport
+        protocol = MagicMock()
+        protocol._writer = writer
+        protocol._reader = reader
+        conn = MagicMock()
+        conn._protocol = protocol
+        return conn
+
+    def test_returns_true_when_protocol_is_none(self) -> None:
+        from dqliteclient.pool import _socket_looks_dead
+
+        conn = MagicMock()
+        conn._protocol = None
+        assert _socket_looks_dead(conn) is True
+
+    def test_returns_false_when_transport_and_reader_both_alive(self) -> None:
+        from dqliteclient.pool import _socket_looks_dead
+
+        transport = MagicMock()
+        transport.is_closing = MagicMock(return_value=False)
+        reader = MagicMock()
+        reader.at_eof = MagicMock(return_value=False)
+
+        conn = self._make_conn(transport=transport, reader=reader)
+        assert _socket_looks_dead(conn) is False
+
+    def test_returns_true_when_transport_is_closing(self) -> None:
+        from dqliteclient.pool import _socket_looks_dead
+
+        transport = MagicMock()
+        transport.is_closing = MagicMock(return_value=True)
+        reader = MagicMock()
+        reader.at_eof = MagicMock(return_value=False)
+
+        conn = self._make_conn(transport=transport, reader=reader)
+        assert _socket_looks_dead(conn) is True
+
+    def test_returns_true_when_reader_at_eof(self) -> None:
+        from dqliteclient.pool import _socket_looks_dead
+
+        transport = MagicMock()
+        transport.is_closing = MagicMock(return_value=False)
+        reader = MagicMock()
+        reader.at_eof = MagicMock(return_value=True)
+
+        conn = self._make_conn(transport=transport, reader=reader)
+        assert _socket_looks_dead(conn) is True
+
+    def test_attribute_error_falls_through_as_alive(self) -> None:
+        """Mocks missing ``is_closing`` / ``at_eof`` default to "assume alive"."""
+        from dqliteclient.pool import _socket_looks_dead
+
+        transport = object()  # no is_closing attribute
+        reader = object()  # no at_eof attribute
+        conn = self._make_conn(transport=transport, reader=reader)
+        assert _socket_looks_dead(conn) is False
+
+    def test_unexpected_exception_is_not_swallowed(self) -> None:
+        """A ``ValueError`` from a broken mock must propagate, not be suppressed."""
+        from dqliteclient.pool import _socket_looks_dead
+
+        transport = MagicMock()
+        transport.is_closing = MagicMock(side_effect=ValueError("broken mock"))
+        reader = MagicMock()
+        reader.at_eof = MagicMock(return_value=False)
+
+        conn = self._make_conn(transport=transport, reader=reader)
+        with pytest.raises(ValueError, match="broken mock"):
+            _socket_looks_dead(conn)