Skip to content

Commit 4652d02

Browse files
Log rollback failure in AsyncAdaptedConnection.close
The narrow ``contextlib.suppress`` used to swallow the categories a best-effort rollback can legitimately raise (OperationalError, InterfaceError, DqliteConnectionError, OSError, TimeoutError, ConnectionError). That kept programming bugs surfacing, but also hid the suppressed rollback failures from operators — a SQLAlchemy-pooled connection returning during a leader flip left no log breadcrumb for the dangling server-side transaction. Replace with a try/except/DEBUG-log-before-continuing, matching the ISSUE-255/257 precedent. The narrow exception tuple is preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 412e030 commit 4652d02

2 files changed

Lines changed: 136 additions & 3 deletions

File tree

src/sqlalchemydqlite/aio.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Async dqlite dialect for SQLAlchemy."""
22

33
import contextlib
4+
import logging
45
import types
56
from collections import deque
67
from collections.abc import Iterable, Iterator, Sequence
@@ -15,6 +16,8 @@
1516
from dqlitedbapi.exceptions import InterfaceError, NotSupportedError, OperationalError
1617
from sqlalchemydqlite.base import DqliteDialect
1718

19+
logger = logging.getLogger(__name__)
20+
1821
if TYPE_CHECKING:
1922
from dqlitedbapi.aio import AsyncConnection
2023

@@ -228,15 +231,24 @@ def close(self) -> None:
228231
# so programming bugs (AttributeError, TypeError, bare RuntimeError,
229232
# etc.) still propagate. The tuple mirrors the client layer's own
230233
# is_disconnect classification in base.py.
231-
with contextlib.suppress(
234+
try:
235+
await_only(self._connection.rollback())
236+
except (
232237
OperationalError,
233238
InterfaceError,
234239
DqliteConnectionError,
235240
OSError,
236241
TimeoutError,
237242
ConnectionError,
238-
):
239-
await_only(self._connection.rollback())
243+
) as exc:
244+
# Silent suppression used to hide e.g. "leader flip
245+
# mid-rollback" from operators — a DEBUG line preserves the
246+
# diagnostic without masking or propagating.
247+
logger.debug(
248+
"AsyncAdaptedConnection.close: rollback failed (%s); proceeding to close",
249+
type(exc).__name__,
250+
exc_info=True,
251+
)
240252
await_only(self._connection.close())
241253

242254

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
"""Pin the DEBUG-log breadcrumb on AsyncAdaptedConnection.close.
2+
3+
The narrow ``contextlib.suppress`` used to hide failed rollbacks
4+
(leader flip, transport timeout, etc.) silently. Replace with a
5+
try/except that DEBUG-logs before continuing to the close. Programming
6+
bugs (AttributeError, RuntimeError, etc.) still propagate.
7+
"""
8+
9+
from __future__ import annotations
10+
11+
import logging
12+
13+
import pytest
14+
15+
from dqliteclient.exceptions import DqliteConnectionError
16+
from dqlitedbapi.exceptions import OperationalError
17+
from sqlalchemydqlite.aio import AsyncAdaptedConnection
18+
19+
20+
class _FakeAsyncConn:
21+
"""A synchronous stand-in: sqlalchemy.util.await_only accepts a plain
22+
callable's return when the adapter is driven by greenlet glue. For
23+
testing we short-circuit to a sync method and let ``await_only``
24+
see the already-complete coroutine-ish value.
25+
"""
26+
27+
def __init__(self, rollback_exc: BaseException | None) -> None:
28+
self._rollback_exc = rollback_exc
29+
self.close_calls = 0
30+
31+
async def rollback(self) -> None:
32+
if self._rollback_exc is not None:
33+
raise self._rollback_exc
34+
35+
async def close(self) -> None:
36+
self.close_calls += 1
37+
38+
39+
def test_close_logs_rollback_failure(caplog: pytest.LogCaptureFixture) -> None:
40+
fake = _FakeAsyncConn(OperationalError("server gone"))
41+
adapter = AsyncAdaptedConnection(fake) # type: ignore[arg-type]
42+
43+
# sqlalchemy.util.await_only drives the coroutine; using a live
44+
# engine is overkill for an observability test. Stub await_only.
45+
from sqlalchemydqlite import aio as aio_module
46+
47+
def _fake_await_only(coro: object) -> object:
48+
# ``coro`` is a coroutine object; run it synchronously via send.
49+
import asyncio
50+
51+
return asyncio.new_event_loop().run_until_complete(coro) # type: ignore[arg-type]
52+
53+
orig = aio_module.await_only
54+
aio_module.await_only = _fake_await_only # type: ignore[assignment]
55+
try:
56+
with caplog.at_level(logging.DEBUG, logger="sqlalchemydqlite.aio"):
57+
adapter.close()
58+
finally:
59+
aio_module.await_only = orig # type: ignore[assignment]
60+
61+
matching = [
62+
r
63+
for r in caplog.records
64+
if r.levelno == logging.DEBUG and "rollback failed" in r.getMessage()
65+
]
66+
assert matching, f"expected DEBUG 'rollback failed' record; got {caplog.records!r}"
67+
assert matching[0].exc_info is not None
68+
assert isinstance(matching[0].exc_info[1], OperationalError)
69+
assert fake.close_calls == 1 # close still ran
70+
71+
72+
def test_close_propagates_programming_bug() -> None:
73+
"""RuntimeError / AttributeError / etc. are NOT suppressed."""
74+
fake = _FakeAsyncConn(RuntimeError("programming bug"))
75+
adapter = AsyncAdaptedConnection(fake) # type: ignore[arg-type]
76+
77+
from sqlalchemydqlite import aio as aio_module
78+
79+
def _fake_await_only(coro: object) -> object:
80+
import asyncio
81+
82+
return asyncio.new_event_loop().run_until_complete(coro) # type: ignore[arg-type]
83+
84+
orig = aio_module.await_only
85+
aio_module.await_only = _fake_await_only # type: ignore[assignment]
86+
try:
87+
with pytest.raises(RuntimeError, match="programming bug"):
88+
adapter.close()
89+
finally:
90+
aio_module.await_only = orig # type: ignore[assignment]
91+
92+
93+
def test_close_with_also_failing_transport_errors(caplog: pytest.LogCaptureFixture) -> None:
94+
"""The full category tuple (OSError, TimeoutError, ConnectionError, etc.)
95+
is all caught and DEBUG-logged.
96+
"""
97+
fake = _FakeAsyncConn(DqliteConnectionError("peer reset"))
98+
adapter = AsyncAdaptedConnection(fake) # type: ignore[arg-type]
99+
100+
from sqlalchemydqlite import aio as aio_module
101+
102+
def _fake_await_only(coro: object) -> object:
103+
import asyncio
104+
105+
return asyncio.new_event_loop().run_until_complete(coro) # type: ignore[arg-type]
106+
107+
orig = aio_module.await_only
108+
aio_module.await_only = _fake_await_only # type: ignore[assignment]
109+
try:
110+
with caplog.at_level(logging.DEBUG, logger="sqlalchemydqlite.aio"):
111+
adapter.close()
112+
finally:
113+
aio_module.await_only = orig # type: ignore[assignment]
114+
115+
matching = [
116+
r
117+
for r in caplog.records
118+
if r.levelno == logging.DEBUG and "rollback failed" in r.getMessage()
119+
]
120+
assert matching
121+
assert fake.close_calls == 1

0 commit comments

Comments
 (0)