From: Mike Bayer Date: Thu, 28 Jan 2016 20:01:31 +0000 (-0500) Subject: - revert the change first made in a6fe4dc, as we are now generalizing X-Git-Tag: rel_1_1_0b1~98^2~65 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8a1e619fb20df1be6ad2e0c563e451e17eb17628;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git - revert the change first made in a6fe4dc, as we are now generalizing the warning here to all safe_reraise() cases in Python 2. - 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. fixes #2696 --- diff --git a/doc/build/changelog/changelog_10.rst b/doc/build/changelog/changelog_10.rst index a0b1ad9572..781911d650 100644 --- a/doc/build/changelog/changelog_10.rst +++ b/doc/build/changelog/changelog_10.rst @@ -19,6 +19,29 @@ :version: 1.0.12 :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 56513860a9..0af8f60fbc 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 11aa9384d3..a1780536a7 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 6d162ff4d7..53dbc12f9f 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 7f8a7c97cd..c81a7580f0 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 7efb5942b4..c7b3315d95 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 522a376e00..abc8ad5c20 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):