- A DBAPI that raises an error on ``connect()`` which is not a subclass

of dbapi.Error (such as ``TypeError``, ``NotImplementedError``, etc.)
will propagate the exception unchanged.  Previously,
the error handling specific to the ``connect()`` routine would both
inappropriately run the exception through the dialect's
:meth:`.Dialect.is_disconnect` routine as well as wrap it in
a :class:`sqlalchemy.exc.DBAPIError`.  It is now propagated unchanged
in the same way as occurs within the execute process. [ticket:2881]
- add tests for this in test_parseconnect, but also add tests in test_execute
to ensure the execute() behavior as well

diff --git a/doc/build/changelog/changelog_08.rst b/doc/build/changelog/changelog_08.rst
index 4c9cc85d9352c71c5cc02b12e506ab80deb3f212..0902ff7e464ef8d8063a0935610e31bf9c9e1b35 100644 (file)
--- a/doc/build/changelog/changelog_08.rst
+++ b/doc/build/changelog/changelog_08.rst
@@ -11,6 +11,20 @@
 .. changelog::
     :version: 0.8.4
 
+     .. change::
+        :tags: bug, engine
+        :versions: 0.9.0b2
+        :tickets: 2881
+
+        A DBAPI that raises an error on ``connect()`` which is not a subclass
+        of dbapi.Error (such as ``TypeError``, ``NotImplementedError``, etc.)
+        will propagate the exception unchanged.  Previously,
+        the error handling specific to the ``connect()`` routine would both
+        inappropriately run the exception through the dialect's
+        :meth:`.Dialect.is_disconnect` routine as well as wrap it in
+        a :class:`sqlalchemy.exc.DBAPIError`.  It is now propagated unchanged
+        in the same way as occurs within the execute process.
+
      .. change::
         :tags: bug, engine, pool
         :versions: 0.9.0b2

diff --git a/lib/sqlalchemy/engine/strategies.py b/lib/sqlalchemy/engine/strategies.py
index ab9d370a3ccca6d4f534df3c232728eb6d403692..5a3b2c5af86dcf2e234f6e1c784ca5b3afbcfff0 100644 (file)
--- a/lib/sqlalchemy/engine/strategies.py
+++ b/lib/sqlalchemy/engine/strategies.py
@@ -78,7 +78,7 @@ class DefaultEngineStrategy(EngineStrategy):
             def connect():
                 try:
                     return dialect.connect(*cargs, **cparams)
-                except Exception as e:
+                except dialect.dbapi.Error as e:
                     invalidated = dialect.is_disconnect(e, None, None)
                     util.raise_from_cause(
                         exc.DBAPIError.instance(None, None,

diff --git a/test/engine/test_execute.py b/test/engine/test_execute.py
index 1f7bad31a21f35ff504a1b0e0fdaf17ea8cb238a..7312bb0412c29f6b95863776485b9005ff4ba371 100644 (file)
--- a/test/engine/test_execute.py
+++ b/test/engine/test_execute.py
@@ -203,19 +203,37 @@ class ExecuteTest(fixtures.TestBase):
         finally:
             conn.close()
 
+    @testing.engines.close_open_connections
     def test_exception_wrapping_dbapi(self):
-        def go(conn):
+        conn = testing.db.connect()
+        for _c in testing.db, conn:
             assert_raises_message(
                 tsa.exc.DBAPIError,
                 r"not_a_valid_statement",
-                conn.execute, 'not_a_valid_statement'
+                _c.execute, 'not_a_valid_statement'
             )
-        go(testing.db)
-        conn = testing.db.connect()
-        try:
-            go(conn)
-        finally:
-            conn.close()
+
+    @testing.requires.sqlite
+    def test_exception_wrapping_non_dbapi_error(self):
+        e = create_engine('sqlite://')
+        e.dialect.is_disconnect = is_disconnect = Mock()
+
+        c = e.connect()
+
+        c.connection.cursor = Mock(
+                return_value=Mock(
+                    execute=Mock(
+                            side_effect=TypeError("I'm not a DBAPI error")
+                    ))
+                )
+
+        assert_raises_message(
+            TypeError,
+            "I'm not a DBAPI error",
+            c.execute, "select "
+        )
+        eq_(is_disconnect.call_count, 0)
+
 
     def test_exception_wrapping_non_dbapi_statement(self):
         class MyType(TypeDecorator):

diff --git a/test/engine/test_parseconnect.py b/test/engine/test_parseconnect.py
index 0ae747b9c6a175c256cbfbf5ea7fb8327f715d01..c4d8b8edc60a8e932b4180687603161b31becb13 100644 (file)
--- a/test/engine/test_parseconnect.py
+++ b/test/engine/test_parseconnect.py
@@ -1,4 +1,4 @@
-from sqlalchemy.testing import assert_raises, eq_
+from sqlalchemy.testing import assert_raises, eq_, assert_raises_message
 from sqlalchemy.util.compat import configparser, StringIO
 import sqlalchemy.engine.url as url
 from sqlalchemy import create_engine, engine_from_config, exc, pool
@@ -256,17 +256,38 @@ pool_timeout=10
 
     @testing.requires.sqlite
     def test_wraps_connect_in_dbapi(self):
-        # sqlite uses SingletonThreadPool which doesnt have max_overflow
+        e = create_engine('sqlite://')
+        sqlite3 = e.dialect.dbapi
 
-        assert_raises(TypeError, create_engine, 'sqlite://',
-                      max_overflow=5, module=mock_sqlite_dbapi)
-        e = create_engine('sqlite://', connect_args={'use_unicode'
-                          : True}, convert_unicode=True)
+        dbapi = MockDBAPI()
+        dbapi.Error = sqlite3.Error,
+        dbapi.ProgrammingError = sqlite3.ProgrammingError
+        dbapi.connect = Mock(side_effect=sqlite3.ProgrammingError("random error"))
         try:
-            e.connect()
+            create_engine('sqlite://', module=dbapi).connect()
+            assert False
         except tsa.exc.DBAPIError as de:
             assert not de.connection_invalidated
 
+
+    @testing.requires.sqlite
+    def test_dont_touch_non_dbapi_exception_on_connect(self):
+        e = create_engine('sqlite://')
+        sqlite3 = e.dialect.dbapi
+
+        dbapi = MockDBAPI()
+        dbapi.Error = sqlite3.Error,
+        dbapi.ProgrammingError = sqlite3.ProgrammingError
+        dbapi.connect = Mock(side_effect=TypeError("I'm not a DBAPI error"))
+        e = create_engine('sqlite://', module=dbapi)
+        e.dialect.is_disconnect = is_disconnect = Mock()
+        assert_raises_message(
+            TypeError,
+            "I'm not a DBAPI error",
+            e.connect
+        )
+        eq_(is_disconnect.call_count, 0)
+
     def test_ensure_dialect_does_is_disconnect_no_conn(self):
         """test that is_disconnect() doesn't choke if no connection, cursor given."""
         dialect = testing.db.dialect