Skip to content

Commit 9dcca52

Browse files
fix: invalidate connection after a failed COMMIT
A non-leader OperationalError from COMMIT leaves server-side state ambiguous (maybe committed, maybe not, maybe still open). The previous code attempted a suppressed ROLLBACK and then cleared _in_transaction, letting the pool recycle a connection with unknown state. Distinguish pre-COMMIT body failures (try ROLLBACK, keep the connection) from COMMIT failures (invalidate so the pool discards it). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6e09702 commit 9dcca52

2 files changed

Lines changed: 33 additions & 3 deletions

File tree

src/dqliteclient/connection.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -338,13 +338,23 @@ async def transaction(self) -> AsyncIterator[None]:
338338
self._in_transaction = False
339339
raise
340340

341+
commit_attempted = False
341342
try:
342343
yield
344+
commit_attempted = True
343345
await self.execute("COMMIT")
344346
except BaseException:
345-
# Swallow rollback failure; original exception is more important
346-
with contextlib.suppress(BaseException):
347-
await self.execute("ROLLBACK")
347+
if commit_attempted:
348+
# COMMIT was sent but failed. Server-side state is ambiguous
349+
# (maybe committed, maybe still open, maybe rolled back). We
350+
# cannot safely reuse this connection — invalidate so the
351+
# pool discards it instead of recycling an unknown state.
352+
self._invalidate()
353+
else:
354+
# Body raised before COMMIT; try to roll back. Swallow
355+
# rollback errors; the original exception is more important.
356+
with contextlib.suppress(BaseException):
357+
await self.execute("ROLLBACK")
348358
raise
349359
finally:
350360
self._tx_owner = None

tests/test_connection.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -821,6 +821,26 @@ async def test_close_on_pool_released_connection_is_noop(self, connected_connect
821821
# Would previously raise InterfaceError("returned to the pool").
822822
await conn.close()
823823

824+
async def test_commit_failure_invalidates_connection(self, connected_connection) -> None:
825+
"""A failed COMMIT leaves server-side state ambiguous; the connection
826+
must be invalidated so the pool doesn't recycle it to another caller
827+
in an unknown state.
828+
"""
829+
from dqliteclient.exceptions import OperationalError as OpError
830+
from dqlitewire.messages import FailureResponse, ResultResponse
831+
832+
conn, mock_reader, _ = connected_connection
833+
mock_reader.read.side_effect = [
834+
ResultResponse(last_insert_id=0, rows_affected=0).encode(), # BEGIN
835+
FailureResponse(code=1, message="disk I/O error").encode(), # COMMIT
836+
# ROLLBACK would also fail on an invalidated conn:
837+
FailureResponse(code=1, message="should not reach").encode(),
838+
]
839+
with pytest.raises(OpError):
840+
async with conn.transaction():
841+
pass
842+
assert not conn.is_connected, "failed COMMIT must invalidate the connection"
843+
824844
async def test_cross_event_loop_raises_interface_error(self) -> None:
825845
"""Using a connection from a different event loop must raise InterfaceError."""
826846
import asyncio

0 commit comments

Comments
 (0)