include rollback exception in closed transaction context manager error message

Improved the error message raised when a Session is used inside a context
manager after the transaction has been rolled back due to an exception.
The InvalidRequestError now includes the original exception that triggered
the rollback, making it clearer why the transaction is no longer active.
Pull request courtesy Ilan Keshet.

Closes: #11297
Pull-request: https://github.com/sqlalchemy/sqlalchemy/pull/11297
Pull-request-sha: 9feb9658078e1acfbeed29a119c9765cf684f677

Change-Id: If4ee3e9f673adfec533726b9b4d5c125dfcf2b9c

diff --git a/doc/build/changelog/unreleased_21/11297.rst b/doc/build/changelog/unreleased_21/11297.rst
new file mode 100644 (file)
index 0000000..f18cb7e
--- /dev/null
+++ b/doc/build/changelog/unreleased_21/11297.rst
@@ -0,0 +1,9 @@
+.. change::
+    :tags: usecase, orm
+    :tickets: 11297
+
+    Improved the error message raised when a :class:`.Session` is used
+    inside a context manager after the transaction has been rolled back due
+    to an exception. The ``InvalidRequestError`` now includes the original
+    exception that triggered the rollback, making it clearer why the
+    transaction is no longer active. Pull request courtesy Ilan Keshet.

diff --git a/lib/sqlalchemy/engine/util.py b/lib/sqlalchemy/engine/util.py
index b4cd054db8c1d00eb3a6c98867226fbd6a664632..8cbfe4b029bbc130f9174ac8211e008f4bc01d41 100644 (file)
--- a/lib/sqlalchemy/engine/util.py
+++ b/lib/sqlalchemy/engine/util.py
@@ -56,6 +56,7 @@ class TransactionalContext:
     __slots__ = ("_outer_trans_ctx", "_trans_subject", "__weakref__")
 
     _trans_subject: Optional[_TConsSubject]
+    _rollback_exception: Optional[BaseException] = None
 
     def _transaction_is_active(self) -> bool:
         raise NotImplementedError()
@@ -95,13 +96,20 @@ class TransactionalContext:
     @classmethod
     def _trans_ctx_check(cls, subject: _TConsSubject) -> None:
         trans_context = subject._trans_context_manager
-        if trans_context:
-            if not trans_context._transaction_is_active():
-                raise exc.InvalidRequestError(
-                    "Can't operate on closed transaction inside context "
-                    "manager.  Please complete the context manager "
-                    "before emitting further commands."
+        if trans_context and not trans_context._transaction_is_active():
+            rollback_exc = trans_context._rollback_exception
+            raise exc.InvalidRequestError(
+                "Can't operate on closed transaction inside context "
+                "manager.  "
+                + (
+                    "The transaction was rolled back due to an "
+                    f"exception: {rollback_exc}.  "
+                    if rollback_exc is not None
+                    else ""
                 )
+                + "Please complete the context manager before "
+                "emitting further commands."
+            )
 
     def __enter__(self) -> Self:
         subject = self._get_subject()

diff --git a/test/orm/test_transaction.py b/test/orm/test_transaction.py
index 4c7487e636fb927674f7dbdad13cd2ae590ead6e..4709d85639328d7218bc8f480a90b878c008284b 100644 (file)
--- a/test/orm/test_transaction.py
+++ b/test/orm/test_transaction.py
@@ -2,6 +2,7 @@ from __future__ import annotations
 
 import contextlib
 import random
+import re
 from typing import Optional
 from typing import TYPE_CHECKING
 
@@ -30,6 +31,7 @@ from sqlalchemy.testing import assert_raises_message
 from sqlalchemy.testing import assert_warnings
 from sqlalchemy.testing import engines
 from sqlalchemy.testing import eq_
+from sqlalchemy.testing import expect_raises
 from sqlalchemy.testing import expect_raises_message
 from sqlalchemy.testing import expect_warnings
 from sqlalchemy.testing import fixtures
@@ -2244,6 +2246,33 @@ class ContextManagerPlusFutureTest(FixtureTest):
                 elif check_operation == "delete":
                     session.delete(u1)
 
+    def test_rollback_exception_in_ctxmanager_error_message(self):
+        """test that when a flush-level exception causes the transaction to
+        be rolled back inside a context manager, subsequent session use raises
+        an error that includes the original exception. #11297"""
+        users, User = self.tables.users, self.classes.User
+
+        self.mapper_registry.map_imperatively(User, users)
+
+        sess = fixture_session()
+
+        with expect_raises(sa_exc.DBAPIError):
+            with sess.begin():
+                sess.add(User())  # name can't be null
+                try:
+                    sess.flush()
+                except sa_exc.DBAPIError as flush_err:
+                    with expect_raises_message(
+                        sa_exc.InvalidRequestError,
+                        "Can't operate on closed transaction inside context "
+                        "manager.  The transaction was rolled back due to "
+                        f"an exception: {re.escape(str(flush_err))}.  "
+                        "Please complete the context manager before "
+                        "emitting further commands.",
+                    ):
+                        sess.connection()
+                    raise
+
 
 class TransactionFlagsTest(fixtures.TestBase):
     def test_in_transaction(self):