Skip to content

Commit dabac9e

Browse files
Delegate 'no active transaction' swallow to the DBAPI layer
The do_commit / do_rollback overrides used a brittle substring match on 'no transaction is active' to swallow the server's error when the connection had no open tx. That workaround now lives inside the dbapi's commit() / rollback() themselves (matching stdlib sqlite3 semantics), so the dialect no longer needs an override — it inherits the parent dialect's default delegation to the DBAPI. Keeps one dialect-level test that verifies real rollback errors (e.g. "database is locked") still propagate through the dialect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent b2bfcbe commit dabac9e

2 files changed

Lines changed: 14 additions & 99 deletions

File tree

src/sqlalchemydqlite/base.py

Lines changed: 5 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -110,41 +110,11 @@ def set_isolation_level(self, dbapi_connection: DBAPIConnection, level: str | No
110110
stacklevel=2,
111111
)
112112

113-
def do_rollback(self, dbapi_connection: DBAPIConnection) -> None:
114-
"""Rollback the current transaction.
115-
116-
dqlite throws an error if we try to rollback when no transaction
117-
is active, so we catch and ignore that specific error.
118-
"""
119-
import dqliteclient.exceptions
120-
import dqlitedbapi.exceptions
121-
122-
try:
123-
dbapi_connection.rollback()
124-
except (
125-
dqlitedbapi.exceptions.OperationalError,
126-
dqliteclient.exceptions.OperationalError,
127-
) as e:
128-
if "no transaction is active" not in str(e).lower():
129-
raise
130-
131-
def do_commit(self, dbapi_connection: DBAPIConnection) -> None:
132-
"""Commit the current transaction.
133-
134-
dqlite throws an error if we try to commit when no transaction
135-
is active, so we catch and ignore that specific error.
136-
"""
137-
import dqliteclient.exceptions
138-
import dqlitedbapi.exceptions
139-
140-
try:
141-
dbapi_connection.commit()
142-
except (
143-
dqlitedbapi.exceptions.OperationalError,
144-
dqliteclient.exceptions.OperationalError,
145-
) as e:
146-
if "no transaction is active" not in str(e).lower():
147-
raise
113+
# do_rollback / do_commit are intentionally left inherited from the
114+
# parent dialect. The "cannot commit/rollback — no transaction is
115+
# active" error is swallowed at the DBAPI layer (dqlitedbapi's
116+
# Connection.commit / rollback), so the dialect doesn't need its own
117+
# workaround. Matches stdlib sqlite3 semantics.
148118

149119
_dqlite_disconnect_messages = (
150120
"Connection closed",

tests/test_dialect.py

Lines changed: 9 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -153,70 +153,15 @@ def test_async_dialect_returns_underlying_connection(self) -> None:
153153

154154

155155
class TestDoRollbackCommit:
156-
def test_do_rollback_catches_only_operational_error(self) -> None:
157-
"""do_rollback should only catch OperationalError, not bare Exception."""
158-
import ast
159-
import inspect
160-
import textwrap
161-
162-
source = textwrap.dedent(inspect.getsource(DqliteDialect.do_rollback))
163-
tree = ast.parse(source)
164-
165-
for node in ast.walk(tree):
166-
if isinstance(node, ast.ExceptHandler):
167-
assert node.type is not None, "do_rollback uses bare except"
168-
if isinstance(node.type, ast.Name):
169-
assert node.type.id != "Exception", (
170-
"do_rollback catches bare Exception; "
171-
"should catch a specific dqlite exception type"
172-
)
173-
174-
def test_do_commit_catches_only_operational_error(self) -> None:
175-
"""do_commit should only catch OperationalError, not bare Exception."""
176-
import ast
177-
import inspect
178-
import textwrap
179-
180-
source = textwrap.dedent(inspect.getsource(DqliteDialect.do_commit))
181-
tree = ast.parse(source)
182-
183-
for node in ast.walk(tree):
184-
if isinstance(node, ast.ExceptHandler):
185-
assert node.type is not None, "do_commit uses bare except"
186-
if isinstance(node.type, ast.Name):
187-
assert node.type.id != "Exception", (
188-
"do_commit catches bare Exception; "
189-
"should catch a specific dqlite exception type"
190-
)
191-
192-
def test_do_rollback_suppresses_no_transaction_error(self) -> None:
193-
"""do_rollback should suppress 'no transaction is active' errors."""
194-
from unittest.mock import MagicMock
195-
196-
import dqlitedbapi.exceptions
197-
198-
dialect = DqliteDialect()
199-
mock_conn = MagicMock()
200-
mock_conn.rollback.side_effect = dqlitedbapi.exceptions.OperationalError(
201-
"no transaction is active"
202-
)
203-
dialect.do_rollback(mock_conn)
204-
205-
def test_do_rollback_suppresses_client_no_transaction_error(self) -> None:
206-
"""do_rollback should suppress dqliteclient 'no transaction' errors too."""
207-
from unittest.mock import MagicMock
208-
209-
import dqliteclient.exceptions
210-
211-
dialect = DqliteDialect()
212-
mock_conn = MagicMock()
213-
mock_conn.rollback.side_effect = dqliteclient.exceptions.OperationalError(
214-
1, "cannot rollback - no transaction is active"
215-
)
216-
dialect.do_rollback(mock_conn)
217-
218-
def test_do_rollback_raises_other_errors(self) -> None:
219-
"""do_rollback should re-raise errors other than 'no transaction'."""
156+
"""The 'no transaction is active' swallow moved down to the DBAPI
157+
layer (python-dqlite-dbapi). The dialect now inherits do_commit /
158+
do_rollback from the parent and delegates straight to the DBAPI,
159+
which handles the no-active-tx case. These tests verify the dialect
160+
doesn't wrap the DBAPI's errors when a real problem occurs.
161+
"""
162+
163+
def test_do_rollback_propagates_real_errors(self) -> None:
164+
"""Real (not 'no transaction') errors propagate through the dialect."""
220165
from unittest.mock import MagicMock
221166

222167
import pytest

0 commit comments

Comments
 (0)