Skip to content

Commit b0a6ef2

Browse files
Narrow connect-cleanup suppress in ClusterClient.connect to match pool's shielded-cleanup discipline
The ``try_connect`` cleanup arm wrapped ``await asyncio.shield(conn.close())`` in a ``contextlib.suppress(BaseException)``. The wide width swallowed everything the shielded close could raise — including ``KeyboardInterrupt`` / ``SystemExit`` (signal shutdowns), programming-bug class exceptions like ``AttributeError``, and any unexpected ``Exception`` — and the bare ``raise`` after the suppress re-raised the original handshake exception, supplanting the new signal entirely. Mirror the pool's canonical pattern at ``pool.py:1018-1042``: * ``asyncio.CancelledError`` is absorbed (asyncio re- delivers at the next await; the bare ``raise`` re- delivers the original handshake exception, so the outer-cancel signal is not lost — just delayed by one await boundary). * ``OSError`` / ``DqliteConnectionError`` are caught and logged (transport-class teardown failures on a half- built conn are expected). * ``KI`` / ``SystemExit`` / other ``Exception`` subclasses propagate — these are the cases the wide suppress was silently dropping. The cleanup arm's intent (drain the inner client's ``_pending_drain`` task before the half-built conn falls out of scope) is preserved; only the failure-mode width changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent bb87ee2 commit b0a6ef2

2 files changed

Lines changed: 87 additions & 2 deletions

File tree

src/dqliteclient/cluster.py

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -584,8 +584,31 @@ async def try_connect() -> DqliteConnection:
584584
# drain task is GC'd mid-flight, producing a
585585
# "Task was destroyed but it is pending" warning
586586
# at interpreter shutdown.
587-
with contextlib.suppress(BaseException):
588-
await asyncio.shield(conn.close())
587+
#
588+
# Mirrors ``pool.py``'s shielded-cleanup
589+
# discipline:
590+
#
591+
# * CancelledError absorbed (canonical asyncio
592+
# pattern — asyncio re-delivers at the next
593+
# await; the bare ``raise`` below re-delivers
594+
# the original handshake exception).
595+
# * OSError / DqliteConnectionError caught and
596+
# logged (transport-class teardown failures
597+
# on a half-built conn are expected).
598+
# * KI / SystemExit / unexpected Exception
599+
# subclasses propagate — a wide
600+
# ``suppress(BaseException)`` here would
601+
# silently swallow signal-class shutdowns and
602+
# programming bugs alike, supplanting the
603+
# original handshake exception.
604+
try:
605+
with contextlib.suppress(asyncio.CancelledError):
606+
await asyncio.shield(conn.close())
607+
except (OSError, DqliteConnectionError):
608+
logger.debug(
609+
"ClusterClient.connect cleanup: conn.close() failed",
610+
exc_info=True,
611+
)
589612
raise
590613
return conn
591614
except (OSError, DqliteConnectionError, ClusterError) as exc:
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
"""Pin: ``ClusterClient.connect``'s try_connect cleanup arm
2+
must NOT swallow unexpected exceptions raised by the
3+
shielded ``conn.close()``. The wide
4+
``contextlib.suppress(BaseException)`` catches a
5+
programming-bug class like ``AttributeError`` that the
6+
narrow canonical pattern (``suppress(asyncio.CancelledError)``
7+
plus a transport-class except) deliberately lets propagate.
8+
9+
Mirrors the pool's discipline at
10+
``pool.py:1018-1042``:
11+
12+
* ``CancelledError``: absorbed (asyncio re-delivers at next
13+
await; the bare ``raise`` re-delivers the original
14+
handshake exception).
15+
* ``OSError`` / ``DqliteConnectionError``: caught and logged
16+
(transport-class teardown failures on a half-built conn
17+
are expected).
18+
* anything else (KI / SystemExit / unexpected ``Exception``
19+
subclasses): propagate.
20+
"""
21+
22+
from __future__ import annotations
23+
24+
import pytest
25+
26+
from dqliteclient.cluster import ClusterClient
27+
28+
29+
@pytest.mark.asyncio
30+
async def test_connect_cleanup_arm_does_not_swallow_unexpected_exception() -> None:
31+
cluster = ClusterClient.from_addresses(["localhost:9001"], timeout=0.5)
32+
33+
async def _fake_find_leader(**kwargs: object) -> str:
34+
return "localhost:9001"
35+
36+
cluster.find_leader = _fake_find_leader # type: ignore[method-assign]
37+
38+
import dqliteclient.cluster as cluster_mod
39+
40+
real_dc = cluster_mod.DqliteConnection
41+
42+
class _StubDqliteConnection:
43+
def __init__(self, *a: object, **kw: object) -> None:
44+
pass
45+
46+
async def connect(self) -> None:
47+
raise OSError("simulated handshake failure")
48+
49+
async def close(self) -> None:
50+
# Programming-bug class — the wide BaseException
51+
# suppress would silently swallow this; the narrow
52+
# canonical pattern lets it propagate so the real
53+
# source of the bug surfaces.
54+
raise AttributeError("unexpected attribute access in close()")
55+
56+
cluster_mod.DqliteConnection = _StubDqliteConnection # type: ignore[assignment]
57+
58+
try:
59+
with pytest.raises(AttributeError, match="unexpected attribute access"):
60+
await cluster.connect("default")
61+
finally:
62+
cluster_mod.DqliteConnection = real_dc # type: ignore[assignment]

0 commit comments

Comments
 (0)