From: Mike Bayer Date: Mon, 27 Mar 2017 14:52:58 +0000 (-0400) Subject: Add safe_reraise() + warnings only to Connection._autorollback X-Git-Tag: rel_1_1_7~2^2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=08f42a915a40756dc9222077ea41cdd32c079b4b;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git Add safe_reraise() + warnings only to Connection._autorollback Added an exception handler that will warn for the "cause" exception on Py2K when the "autorollback" feature of :class:`.Connection` itself raises an exception. In Py3K, the two exceptions are naturally reported by the interpreter as one occurring during the handling of the other. This is continuing with the series of changes for rollback failure handling that were last visited as part of :ticket:`2696` in 1.0.12. Change-Id: I600ba455a14ebaea27c6189889181f97c632f179 Fixes: #3946 (cherry picked from commit c0a224aba3d4e2a41f92a29f9d18c6cb9d09d61f) --- diff --git a/doc/build/changelog/changelog_11.rst b/doc/build/changelog/changelog_11.rst index c60550923d..69bdd32742 100644 --- a/doc/build/changelog/changelog_11.rst +++ b/doc/build/changelog/changelog_11.rst @@ -21,6 +21,18 @@ .. changelog:: :version: 1.1.7 + .. change:: + :tags: bug, engine + :tickets: 3946 + :versions: 1.2.0b1 + + Added an exception handler that will warn for the "cause" exception on + Py2K when the "autorollback" feature of :class:`.Connection` itself + raises an exception. In Py3K, the two exceptions are naturally reported + by the interpreter as one occurring during the handling of the other. + This is continuing with the series of changes for rollback failure + handling that were last visited as part of :ticket:`2696` in 1.0.12. + .. change:: :tags: bug, orm :tickets: 3947 diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py index 0334d2d7a8..f680edadaa 100644 --- a/lib/sqlalchemy/engine/base.py +++ b/lib/sqlalchemy/engine/base.py @@ -1383,7 +1383,8 @@ class Connection(Connectable): if not self._is_disconnect: if cursor: self._safe_close_cursor(cursor) - self._autorollback() + with util.safe_reraise(warn_only=True): + self._autorollback() if newraise: util.raise_from_cause(newraise, exc_info) diff --git a/lib/sqlalchemy/testing/assertions.py b/lib/sqlalchemy/testing/assertions.py index 44551bd863..3ee38937f7 100644 --- a/lib/sqlalchemy/testing/assertions.py +++ b/lib/sqlalchemy/testing/assertions.py @@ -10,11 +10,11 @@ from __future__ import absolute_import from . import util as testutil from sqlalchemy import pool, orm, util from sqlalchemy.engine import default, url -from sqlalchemy.util import decorator +from sqlalchemy.util import decorator, compat from sqlalchemy import types as sqltypes, schema, exc as sa_exc import warnings import re -from .exclusions import db_spec, _is_excluded +from .exclusions import db_spec from . import assertsql from . import config from .util import fail @@ -118,7 +118,8 @@ def uses_deprecated(*messages): @contextlib.contextmanager -def _expect_warnings(exc_cls, messages, regex=True, assert_=True): +def _expect_warnings(exc_cls, messages, regex=True, assert_=True, + py2konly=False): if regex: filters = [re.compile(msg, re.I | re.S) for msg in messages] @@ -147,7 +148,7 @@ def _expect_warnings(exc_cls, messages, regex=True, assert_=True): with mock.patch("warnings.warn", our_warn): yield - if assert_: + if assert_ and (not py2konly or not compat.py3k): assert not seen, "Warnings were not seen: %s" % \ ", ".join("%r" % (s.pattern if regex else s) for s in seen) diff --git a/lib/sqlalchemy/util/langhelpers.py b/lib/sqlalchemy/util/langhelpers.py index 68c0f885b5..66a06c5e9d 100644 --- a/lib/sqlalchemy/util/langhelpers.py +++ b/lib/sqlalchemy/util/langhelpers.py @@ -49,6 +49,11 @@ class safe_reraise(object): """ + __slots__ = ('warn_only', '_exc_info') + + def __init__(self, warn_only=False): + self.warn_only = warn_only + def __enter__(self): self._exc_info = sys.exc_info() @@ -57,7 +62,8 @@ class safe_reraise(object): if type_ is None: exc_type, exc_value, exc_tb = self._exc_info self._exc_info = None # remove potential circular references - compat.reraise(exc_type, exc_value, exc_tb) + if not self.warn_only: + 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 diff --git a/test/engine/test_execute.py b/test/engine/test_execute.py index 32834f950b..4cc101598b 100644 --- a/test/engine/test_execute.py +++ b/test/engine/test_execute.py @@ -1,7 +1,7 @@ # coding: utf-8 from sqlalchemy.testing import eq_, assert_raises, assert_raises_message, \ - config, is_, is_not_, le_ + config, is_, is_not_, le_, expect_warnings import re from sqlalchemy.testing.util import picklers from sqlalchemy.interfaces import ConnectionProxy @@ -1804,6 +1804,26 @@ class HandleErrorTest(fixtures.TestBase): ) eq_(patched.call_count, 1) + def test_exception_autorollback_fails(self): + engine = engines.testing_engine() + conn = engine.connect() + + def boom(connection): + raise engine.dialect.dbapi.OperationalError("rollback failed") + + with expect_warnings( + r"An exception has occurred during handling of a previous " + r"exception. The previous exception is.*i_dont_exist", + py2konly=True + ): + with patch.object(conn.dialect, "do_rollback", boom) as patched: + assert_raises_message( + tsa.exc.OperationalError, + "rollback failed", + conn.execute, + "insert into i_dont_exist (x) values ('y')" + ) + def test_exception_event_ad_hoc_context(self): """test that handle_error is called with a context in cases where _handle_dbapi_error() is normally called without diff --git a/test/engine/test_reconnect.py b/test/engine/test_reconnect.py index f04311790d..db70436e06 100644 --- a/test/engine/test_reconnect.py +++ b/test/engine/test_reconnect.py @@ -1,4 +1,5 @@ -from sqlalchemy.testing import eq_, ne_, assert_raises, assert_raises_message +from sqlalchemy.testing import eq_, ne_, assert_raises, \ + expect_warnings, assert_raises_message import time from sqlalchemy import ( select, MetaData, Integer, String, create_engine, pool, exc, util) @@ -308,11 +309,17 @@ class MockReconnectTest(fixtures.TestBase): self.dbapi.shutdown("rollback_no_disconnect") # raises error - assert_raises_message( - tsa.exc.DBAPIError, - "something broke on rollback but we didn't lose the connection", - conn.execute, select([1]) - ) + with expect_warnings( + "An exception has occurred during handling .*" + "something broke on execute but we didn't lose the connection", + py2konly=True + ): + assert_raises_message( + tsa.exc.DBAPIError, + "something broke on rollback but we didn't " + "lose the connection", + conn.execute, select([1]) + ) assert conn.closed assert not conn.invalidated @@ -333,11 +340,16 @@ class MockReconnectTest(fixtures.TestBase): self.dbapi.shutdown("rollback") # raises error - assert_raises_message( - tsa.exc.DBAPIError, - "Lost the DB connection on rollback", - conn.execute, select([1]) - ) + with expect_warnings( + "An exception has occurred during handling .*" + "something broke on execute but we didn't lose the connection", + py2konly=True + ): + assert_raises_message( + tsa.exc.DBAPIError, + "Lost the DB connection on rollback", + conn.execute, select([1]) + ) assert not conn.closed assert conn.invalidated @@ -348,11 +360,16 @@ class MockReconnectTest(fixtures.TestBase): self.dbapi.shutdown("rollback") # raises error - assert_raises_message( - tsa.exc.DBAPIError, - "Lost the DB connection on rollback", - conn.execute, select([1]) - ) + with expect_warnings( + "An exception has occurred during handling .*" + "something broke on execute but we didn't lose the connection", + py2konly=True + ): + assert_raises_message( + tsa.exc.DBAPIError, + "Lost the DB connection on rollback", + conn.execute, select([1]) + ) assert conn.closed assert conn.invalidated @@ -666,10 +683,14 @@ class RealReconnectTest(fixtures.TestBase): self.engine.dialect.is_disconnect = is_disconnect conn = self.engine.connect() self.engine.test_shutdown() - assert_raises( - tsa.exc.DBAPIError, - conn.execute, select([1]) - ) + with expect_warnings( + "An exception has occurred during handling .*", + py2konly=True + ): + assert_raises( + tsa.exc.DBAPIError, + conn.execute, select([1]) + ) def test_rollback_on_invalid_plain(self): conn = self.engine.connect()