Skip to content

Commit 61ab110

Browse files
Narrow exception handling in do_rollback and do_commit
Catch specific OperationalError types instead of bare Exception to prevent swallowing unrelated errors. Also use case-insensitive string matching and handle both dqlitedbapi and dqliteclient exception types. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent aa4f17c commit 61ab110

2 files changed

Lines changed: 96 additions & 6 deletions

File tree

src/sqlalchemydqlite/base.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,16 @@ def do_rollback(self, dbapi_connection: DBAPIConnection) -> None:
7272
dqlite throws an error if we try to rollback when no transaction
7373
is active, so we catch and ignore that specific error.
7474
"""
75+
import dqliteclient.exceptions
76+
import dqlitedbapi.exceptions
77+
7578
try:
7679
dbapi_connection.rollback()
77-
except Exception as e:
78-
# Ignore "no transaction is active" errors
79-
if "no transaction is active" not in str(e):
80+
except (
81+
dqlitedbapi.exceptions.OperationalError,
82+
dqliteclient.exceptions.OperationalError,
83+
) as e:
84+
if "no transaction is active" not in str(e).lower():
8085
raise
8186

8287
def do_commit(self, dbapi_connection: DBAPIConnection) -> None:
@@ -85,11 +90,16 @@ def do_commit(self, dbapi_connection: DBAPIConnection) -> None:
8590
dqlite throws an error if we try to commit when no transaction
8691
is active, so we catch and ignore that specific error.
8792
"""
93+
import dqliteclient.exceptions
94+
import dqlitedbapi.exceptions
95+
8896
try:
8997
dbapi_connection.commit()
90-
except Exception as e:
91-
# Ignore "no transaction is active" errors
92-
if "no transaction is active" not in str(e):
98+
except (
99+
dqlitedbapi.exceptions.OperationalError,
100+
dqliteclient.exceptions.OperationalError,
101+
) as e:
102+
if "no transaction is active" not in str(e).lower():
93103
raise
94104

95105
_dqlite_disconnect_messages = (

tests/test_dialect.py

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,86 @@ def test_async_dialect_returns_underlying_connection(self) -> None:
152152
)
153153

154154

155+
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'."""
220+
from unittest.mock import MagicMock
221+
222+
import pytest
223+
224+
import dqlitedbapi.exceptions
225+
226+
dialect = DqliteDialect()
227+
mock_conn = MagicMock()
228+
mock_conn.rollback.side_effect = dqlitedbapi.exceptions.OperationalError(
229+
"database is locked"
230+
)
231+
with pytest.raises(dqlitedbapi.exceptions.OperationalError, match="database is locked"):
232+
dialect.do_rollback(mock_conn)
233+
234+
155235
class TestIsDisconnect:
156236
def test_recognizes_connection_closed(self) -> None:
157237
"""is_disconnect should return True for connection-closed errors."""

0 commit comments

Comments
 (0)