- 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 8f30f1a2c7eaeea8f3b4811bde379cd793788886..ec10950b256df5fafa0a0bb1e2c8ff0f9de0c524 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, e:
+                except dialect.dbapi.Error, e:
                     invalidated = dialect.is_disconnect(e, None, None)
                     # Py3K
                     #raise exc.DBAPIError.instance(None, None,

diff --git a/test/engine/test_execute.py b/test/engine/test_execute.py
index daee642c7993b7798b6f36d3eec73533640a42ed..24598af96ca6a4e18a62f4daf1b7b58fe71707ac 100644 (file)
--- a/test/engine/test_execute.py
+++ b/test/engine/test_execute.py
@@ -206,19 +206,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 b9bb3393e70742970f01eb15908e7f6dd8fd20a5..b30e87ad3c9f6a303c0cea86365051df00c080bd 100644 (file)
--- a/test/engine/test_parseconnect.py
+++ b/test/engine/test_parseconnect.py
@@ -235,17 +235,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, 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