diff options
| -rw-r--r-- | doc/build/changelog/changelog_10.rst | 23 | ||||
| -rw-r--r-- | lib/sqlalchemy/orm/session.py | 16 | ||||
| -rw-r--r-- | lib/sqlalchemy/util/langhelpers.py | 7 | ||||
| -rw-r--r-- | test/base/test_utils.py | 34 | ||||
| -rw-r--r-- | test/engine/test_transaction.py | 21 | ||||
| -rw-r--r-- | test/orm/test_transaction.py | 6 | ||||
| -rw-r--r-- | test/requirements.py | 4 |
7 files changed, 93 insertions, 18 deletions
diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index a0b1ad957..781911d65 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -20,6 +20,29 @@ :released: .. change:: + :tags: bug, engine, mysql + :tickets: 2696 + + Revisiting :ticket:`2696`, first released in 1.0.10, which attempts to + work around Python 2's lack of exception context reporting by emitting + a warning for an exception that was interrupted by a second exception + when attempting to roll back the already-failed transaction; this + issue continues to occur for MySQL backends in conjunction with a + savepoint that gets unexpectedly lost, which then causes a + "no such savepoint" error when the rollback is attempted, obscuring + what the original condition was. + + The approach has been generalized to the Core "safe + reraise" function which takes place across the ORM and Core in any + place that a transaction is being rolled back in response to an error + which occurred trying to commit, including the context managers + provided by :class:`.Session` and :class:`.Connection`, and taking + place for operations such as a failure on "RELEASE SAVEPOINT". + Previously, the fix was only in place for a specific path within + the ORM flush/commit process; it now takes place for all transational + context managers as well. + + .. change:: :tags: bug, sql :tickets: 3632 diff --git a/lib/sqlalchemy/orm/session.py b/lib/sqlalchemy/orm/session.py index 56513860a..0af8f60fb 100644 --- a/lib/sqlalchemy/orm/session.py +++ b/lib/sqlalchemy/orm/session.py @@ -408,23 +408,11 @@ class SessionTransaction(object): for subtransaction in stx._iterate_parents(upto=self): subtransaction.close() - if _capture_exception: - captured_exception = sys.exc_info()[1] - boundary = self if self._state in (ACTIVE, PREPARED): for transaction in self._iterate_parents(): if transaction._parent is None or transaction.nested: - try: - transaction._rollback_impl() - except Exception: - if _capture_exception: - util.warn( - "An exception raised during a Session " - "persistence operation cannot be raised " - "due to an additional ROLLBACK exception; " - "the exception is: %s" % captured_exception) - raise + transaction._rollback_impl() transaction._state = DEACTIVE boundary = transaction break @@ -446,7 +434,7 @@ class SessionTransaction(object): self.close() if self._parent and _capture_exception: - self._parent._rollback_exception = captured_exception + self._parent._rollback_exception = sys.exc_info()[1] sess.dispatch.after_soft_rollback(sess, self) diff --git a/lib/sqlalchemy/util/langhelpers.py b/lib/sqlalchemy/util/langhelpers.py index 11aa9384d..a1780536a 100644 --- a/lib/sqlalchemy/util/langhelpers.py +++ b/lib/sqlalchemy/util/langhelpers.py @@ -59,6 +59,13 @@ class safe_reraise(object): self._exc_info = None # remove potential circular references compat.reraise(exc_type, exc_value, exc_tb) else: + if not compat.py3k and self._exc_info and self._exc_info[1]: + # emulate Py3K's behavior of telling us when an exception + # occurs in an exception handler. + warn( + "An exception has occurred during handling of a " + "previous exception. The previous exception " + "is:\n %s %s\n" % (self._exc_info[0], self._exc_info[1])) self._exc_info = None # remove potential circular references compat.reraise(type_, value, traceback) diff --git a/test/base/test_utils.py b/test/base/test_utils.py index 6d162ff4d..53dbc12f9 100644 --- a/test/base/test_utils.py +++ b/test/base/test_utils.py @@ -3,7 +3,7 @@ import sys from sqlalchemy import util, sql, exc, testing from sqlalchemy.testing import assert_raises, assert_raises_message, fixtures -from sqlalchemy.testing import eq_, is_, ne_, fails_if, mock +from sqlalchemy.testing import eq_, is_, ne_, fails_if, mock, expect_warnings from sqlalchemy.testing.util import picklers, gc_collect from sqlalchemy.util import classproperty, WeakSequence, get_callable_argspec from sqlalchemy.sql import column @@ -2192,6 +2192,38 @@ class ReraiseTest(fixtures.TestBase): if testing.requires.python3.enabled: is_(moe.__cause__, me) + @testing.requires.python2 + def test_safe_reraise_py2k_warning(self): + class MyException(Exception): + pass + + class MyOtherException(Exception): + pass + + m1 = MyException("exc one") + m2 = MyOtherException("exc two") + + def go2(): + raise m2 + + def go(): + try: + raise m1 + except: + with util.safe_reraise(): + go2() + + with expect_warnings( + "An exception has occurred during handling of a previous " + "exception. The previous exception " + "is:.*MyException.*exc one" + ): + try: + go() + assert False + except MyOtherException: + pass + class TestClassProperty(fixtures.TestBase): diff --git a/test/engine/test_transaction.py b/test/engine/test_transaction.py index 7f8a7c97c..c81a7580f 100644 --- a/test/engine/test_transaction.py +++ b/test/engine/test_transaction.py @@ -218,6 +218,27 @@ class TransactionTest(fixtures.TestBase): finally: connection.close() + @testing.requires.python2 + @testing.requires.savepoints_w_release + def test_savepoint_release_fails_warning(self): + with testing.db.connect() as connection: + connection.begin() + + with expect_warnings( + "An exception has occurred during handling of a previous " + "exception. The previous exception " + "is:.*..SQL\:.*RELEASE SAVEPOINT" + ): + def go(): + with connection.begin_nested() as savepoint: + connection.dialect.do_release_savepoint( + connection, savepoint._savepoint) + assert_raises_message( + exc.DBAPIError, + ".*SQL\:.*ROLLBACK TO SAVEPOINT", + go + ) + def test_retains_through_options(self): connection = testing.db.connect() try: diff --git a/test/orm/test_transaction.py b/test/orm/test_transaction.py index 7efb5942b..c7b3315d9 100644 --- a/test/orm/test_transaction.py +++ b/test/orm/test_transaction.py @@ -657,8 +657,8 @@ class SessionTransactionTest(FixtureTest): assert session.transaction is not None, \ 'autocommit=False should start a new transaction' - @testing.skip_if("oracle", "oracle doesn't support release of savepoint") - @testing.requires.savepoints + @testing.requires.python2 + @testing.requires.savepoints_w_release def test_report_primary_error_when_rollback_fails(self): User, users = self.classes.User, self.tables.users @@ -666,7 +666,7 @@ class SessionTransactionTest(FixtureTest): session = Session(testing.db) - with expect_warnings(".*due to an additional ROLLBACK.*INSERT INTO"): + with expect_warnings(".*during handling of a previous exception.*"): session.begin_nested() savepoint = session.\ connection()._Connection__transaction._savepoint diff --git a/test/requirements.py b/test/requirements.py index 522a376e0..abc8ad5c2 100644 --- a/test/requirements.py +++ b/test/requirements.py @@ -286,6 +286,10 @@ class DefaultRequirements(SuiteRequirements): ("mysql", "<", (5, 0, 3)), ], "savepoints not supported") + @property + def savepoints_w_release(self): + return self.savepoints + skip_if( + "oracle", "oracle doesn't support release of savepoint") @property def schemas(self): |