allow rollback within _prepare_impl on twophase prepare failure

When tpc_prepare() raised during SessionTransaction._prepare_impl(),
the error handler's call to self.rollback() was blocked by the
@declare_states decorator, which had set _next_state to
CHANGE_IN_PROGRESS. This caused IllegalStateChangeError to be raised
instead of the original database exception, masking the real error
and preventing proper cleanup.

Used _expect_state(SessionTransactionState.CLOSED) to temporarily
allow the rollback state transition, matching the existing pattern
used in commit() for the close() call.

Fixes: #13356
Change-Id: Ie8212d5b6f8515340cf9d83c56dcbfa5a7415812

diff --git a/doc/build/changelog/unreleased_20/13356.rst b/doc/build/changelog/unreleased_20/13356.rst
new file mode 100644 (file)
index 0000000..047d36f
--- /dev/null
+++ b/doc/build/changelog/unreleased_20/13356.rst
@@ -0,0 +1,11 @@
+.. change::
+    :tags: bug, orm
+    :tickets: 13356
+
+    Fixed bug where a failure during ``tpc_prepare()`` within
+    :meth:`_orm.Session.commit` for a two-phase session would raise
+    :class:`.IllegalStateChangeError` instead of the original database
+    exception.  The internal ``_prepare_impl()`` method's error handler
+    was unable to invoke :meth:`_orm.SessionTransaction.rollback` due
+    to a state-change guard, preventing proper cleanup and masking the
+    underlying error.

diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py
index eae68db6e6e96ab030fff02dab62485aca92a707..0089b7b13dcc4c2a491671cab529e7735983d37d 100644 (file)
--- a/lib/sqlalchemy/orm/session.py
+++ b/lib/sqlalchemy/orm/session.py
@@ -1309,7 +1309,8 @@ class SessionTransaction(_StateChange, TransactionalContext):
                     cast("TwoPhaseTransaction", t[1]).prepare()
             except:
                 with util.safe_reraise():
-                    self.rollback()
+                    with self._expect_state(SessionTransactionState.CLOSED):
+                        self.rollback()
 
         self._state = SessionTransactionState.PREPARED
 

diff --git a/test/orm/test_transaction.py b/test/orm/test_transaction.py
index 2443bb784b3ff2bb4a7b5e992862cc798dfd5338..4c7487e636fb927674f7dbdad13cd2ae590ead6e 100644 (file)
--- a/test/orm/test_transaction.py
+++ b/test/orm/test_transaction.py
@@ -1535,6 +1535,26 @@ class TwoPhaseTest(_LocalFixture):
 
         assert u not in s
 
+    @testing.requires.two_phase_transactions
+    def test_rollback_on_prepare_failure(self):
+        """test #13356"""
+
+        User = self.classes.User
+        s = fixture_session(twophase=True)
+
+        u = User(name="ed")
+        s.add(u)
+
+        with mock.patch.object(
+            testing.db.dialect,
+            "do_prepare_twophase",
+            side_effect=Exception("simulated prepare failure"),
+        ):
+            with expect_raises_message(Exception, "simulated prepare failure"):
+                s.commit()
+
+        assert u not in s
+
 
 class RollbackRecoverTest(_LocalFixture):
     __sparse_driver_backend__ = True