Narrow do_ping's cursor.close exception handling

contextlib.suppress(Exception) on the final close call used to hide
every error — including programming bugs like a ValueError or
AttributeError introduced by a refactor inside Cursor.close. That
silently degraded pool pre-ping to "always alive" while looking
functional.

Replace the broad suppress with the same narrow 5-class tuple the
outer execute-level except already uses — OperationalError,
InterfaceError, DqliteConnectionError, OSError, TimeoutError — and
DEBUG-log the suppressed cause so operators can tell close-swallow
from close-success in logs, matching the pattern AsyncAdaptedConnection.close
already uses for rollback failures.

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

Author: antoineleclair

src/sqlalchemydqlite/base.py

@@ -1,7 +1,7 @@
 """Base dqlite dialect for SQLAlchemy."""
 
-import contextlib
 import datetime
+import logging
 import math
 import types
 import warnings
@@ -18,6 +18,8 @@
 import dqlitedbapi.exceptions as _dbapi_exc
 from dqlitewire import LEADER_ERROR_CODES as _LEADER_CHANGE_CODES
 
+logger = logging.getLogger(__name__)
+
 __all__ = ["DqliteDialect"]
 
 _TRUE_TOKENS = frozenset({"1", "true", "yes", "on"})
@@ -372,8 +374,27 @@ def do_ping(self, dbapi_connection: Any) -> bool:
             ):
                 return False
         finally:
-            with contextlib.suppress(Exception):
-                cursor.close()  # cursor close shouldn't crash the ping result
+            # Narrow the suppression to the same set as the outer
+            # except: connection-level errors are expected on a dead
+            # socket and must not crash the ping; programming bugs
+            # (TypeError, AttributeError, ValueError, …) must still
+            # propagate so refactors can't silently break the probe.
+            # DEBUG-log the suppressed cause so operators can tell
+            # close-swallow from close-success in logs.
+            try:
+                cursor.close()
+            except (
+                _dbapi_exc.OperationalError,
+                _dbapi_exc.InterfaceError,
+                _client_exc.DqliteConnectionError,
+                OSError,
+                TimeoutError,
+            ) as exc:
+                logger.debug(
+                    "do_ping: cursor.close failed (%s); proceeding",
+                    type(exc).__name__,
+                    exc_info=True,
+                )
 
     def _get_server_version_info(self, connection: Any) -> tuple[int, ...]:
         """Return the server's SQLite version as a tuple.

tests/test_do_ping_close_narrow.py

@@ -0,0 +1,106 @@
+"""``do_ping`` must not swallow programming bugs from cursor.close.
+
+The inner ``SELECT 1`` / except-connection-errors loop correctly
+narrows to transport-level failures. The ``finally`` used to catch
+every ``Exception`` on ``cursor.close()`` — hiding e.g. a
+``ValueError`` / ``TypeError`` introduced by a refactor — and emit
+nothing to the logs. That is too broad: programming bugs must be
+visible, and legitimate connection-level close failures should leave
+a diagnostic trail.
+
+Peer of ISSUE-297.
+"""
+
+from __future__ import annotations
+
+import logging
+
+import pytest
+
+import dqliteclient.exceptions as _client_exc
+import dqlitedbapi.exceptions as _dbapi_exc
+from sqlalchemydqlite.base import DqliteDialect
+
+
+class _CursorExecuteOK:
+    """Cursor whose execute succeeds; close is configurable per-test."""
+
+    def __init__(self, on_close: Exception | None) -> None:
+        self._on_close = on_close
+
+    def execute(self, sql: str) -> None:
+        pass
+
+    def close(self) -> None:
+        if self._on_close is not None:
+            raise self._on_close
+
+
+class _CursorExecuteRaises:
+    """Cursor whose execute raises a connection-level error."""
+
+    def __init__(self, on_execute: Exception, on_close: Exception | None) -> None:
+        self._on_execute = on_execute
+        self._on_close = on_close
+
+    def execute(self, sql: str) -> None:
+        raise self._on_execute
+
+    def close(self) -> None:
+        if self._on_close is not None:
+            raise self._on_close
+
+
+class _FakeConnection:
+    """Minimal ``dbapi_connection`` shape — only ``cursor()`` is used."""
+
+    def __init__(self, cursor: object) -> None:
+        self._cursor = cursor
+
+    def cursor(self) -> object:
+        return self._cursor
+
+
+def test_do_ping_propagates_programming_bug_from_close() -> None:
+    """A ``ValueError`` from ``cursor.close`` must propagate.
+
+    A bare ``contextlib.suppress(Exception)`` used to hide this,
+    effectively disabling pool pre-ping while looking like it
+    worked.
+    """
+    cursor = _CursorExecuteOK(on_close=ValueError("refactor bug"))
+    conn = _FakeConnection(cursor)
+    dialect = DqliteDialect()
+    with pytest.raises(ValueError, match="refactor bug"):
+        dialect.do_ping(conn)
+
+
+def test_do_ping_swallows_connection_level_close_errors_with_log(
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """A connection-level error on close is expected on a dead socket;
+    it must be swallowed AND DEBUG-logged.
+    """
+    caplog.set_level(logging.DEBUG, logger="sqlalchemydqlite.base")
+    cursor = _CursorExecuteRaises(
+        on_execute=_dbapi_exc.OperationalError("dead"),
+        on_close=_client_exc.DqliteConnectionError("socket closed"),
+    )
+    conn = _FakeConnection(cursor)
+    dialect = DqliteDialect()
+    # Ping returns False for dead connection; does not raise.
+    assert dialect.do_ping(conn) is False
+    # Close-failure is logged at DEBUG.
+    messages = [r.getMessage() for r in caplog.records if r.name == "sqlalchemydqlite.base"]
+    assert any("cursor.close failed" in m for m in messages), messages
+
+
+def test_do_ping_happy_path_is_silent(caplog: pytest.LogCaptureFixture) -> None:
+    """Successful ping: execute succeeds, close succeeds, no DEBUG noise."""
+    caplog.set_level(logging.DEBUG, logger="sqlalchemydqlite.base")
+    cursor = _CursorExecuteOK(on_close=None)
+    conn = _FakeConnection(cursor)
+    dialect = DqliteDialect()
+    assert dialect.do_ping(conn) is True
+    messages = [r.getMessage() for r in caplog.records if r.name == "sqlalchemydqlite.base"]
+    assert not messages, f"happy path must not emit DEBUG logs; got {messages!r}"