fix for #195 (#196)

* fix for #195

* add changes

* - fix fix :)
- add test
- make setup more resilient on OSX

* attempt to fix tests

* fixes

* test tweaks

* more test cleanup

* make tests besides my new one pass

* fix my test

* fix crashing

* reduce changes and fix potential issues

* better fix

* better fixes

* fix test hopefully

* bumps to try to fix crash

* wow is this test nuanced!

* flake fixes

* swap back and disable test

* fix test?

* fix tests

CHANGES.txt

@@ -4,7 +4,7 @@ Changes
 0.3.3 (YYYY-MM-DD)
 ^^^^^^^^^^^^^^^^^^
 * Parameter echo passed properly in cursor #185
-
+* Close bad connections before returning back to pool #195
 
 0.3.2 (2018-08-04)
 ^^^^^^^^^^^^^^^^^^

Makefile

@@ -40,6 +40,8 @@ doc:
 docker_build:
 	make -C ci build
 
+# NOTE: we start crashing if running tests with -n auto
+
 docker_test:
 	docker run --rm -v /$$(pwd):/aioodbc -v /var/run/docker.sock:/var/run/docker.sock --name aioodbc-test-$$(date +%s) --net=host -e PYTHONASYNCIODEBUG=$(PYTHONASYNCIODEBUG) -it jettify/aioodbc-test:latest py.test -sv tests $(FLAGS)
 

aioodbc/connection.py

@@ -6,7 +6,7 @@
 
 import pyodbc
 from .cursor import Cursor
-from .utils import _ContextManager
+from .utils import _ContextManager, _is_conn_close_error
 
 
 __all__ = ['connect', 'Connection']
@@ -158,13 +158,18 @@ async def execute(self, sql, *args):
         by each call, this should not be used if more than one SQL
         statement needs to be executed.
 
-        :param sql: str, formated sql statement
+        :param sql: str, formatted sql statement
         :param args: tuple, arguments for construction of sql statement
         """
-        _cursor = await self._execute(self._conn.execute, sql, *args)
-        connection = self
-        cursor = Cursor(_cursor, connection, echo=self._echo)
-        return cursor
+        try:
+            _cursor = await self._execute(self._conn.execute, sql, *args)
+            connection = self
+            cursor = Cursor(_cursor, connection, echo=self._echo)
+            return cursor
+        except pyodbc.Error as e:
+            if _is_conn_close_error(e):
+                await self.close()
+            raise
 
     def getinfo(self, type_):
         """Returns general information about the driver and data source

aioodbc/cursor.py

@@ -1,6 +1,6 @@
 import pyodbc
 from .log import logger
-from .utils import PY_352
+from .utils import PY_352, _is_conn_close_error
 
 
 __all__ = ['Cursor']
@@ -21,12 +21,18 @@ def __init__(self, pyodbc_cursor, connection, echo=False):
         self._loop = connection.loop
         self._echo = echo
 
-    def _run_operation(self, func, *args, **kwargs):
+    async def _run_operation(self, func, *args, **kwargs):
         # execute func in thread pool of attached to cursor connection
         if not self._conn:
             raise pyodbc.OperationalError('Cursor is closed.')
-        future = self._conn._execute(func, *args, **kwargs)
-        return future
+
+        try:
+            result = await self._conn._execute(func, *args, **kwargs)
+            return result
+        except pyodbc.Error as e:
+            if self._conn and _is_conn_close_error(e):
+                await self._conn.close()
+            raise
 
     @property
     def echo(self):
@@ -118,6 +124,7 @@ async def execute(self, sql, *params):
         if self._echo:
             logger.info(sql)
             logger.info("%r", sql)
+
         await self._run_operation(self._impl.execute, sql, *params)
         return self
 

aioodbc/utils.py

@@ -1,9 +1,37 @@
 import sys
 from collections.abc import Coroutine
 
+from pyodbc import Error
+
 
 PY_352 = sys.version_info >= (3, 5, 2)
 
+# Issue #195.  Don't pollute the pool with bad conns
+# Unfortunately occasionally sqlite will return 'HY000' for invalid query,
+# so we need specialize the check
+_CONN_CLOSE_ERRORS = {
+    # [Microsoft][ODBC Driver 17 for SQL Server]Communication link failure
+    '08S01': None,
+
+    # [HY000] server closed the connection unexpectedly
+    'HY000': '[HY000] server closed the connection unexpectedly',
+}
+
+
+def _is_conn_close_error(e):
+    if not isinstance(e, Error) or len(e.args) < 2:
+        return False
+
+    sqlstate, msg = e.args[0], e.args[1]
+    if sqlstate not in _CONN_CLOSE_ERRORS:
+        return False
+
+    check_msg = _CONN_CLOSE_ERRORS[sqlstate]
+    if not check_msg:
+        return True
+
+    return msg.startswith(check_msg)
+
 
 class _ContextManager(Coroutine):
 
@@ -51,7 +79,7 @@ async def __aenter__(self):
 
     async def __aexit__(self, exc_type, exc, tb):
         if exc_type:
-            self._obj.rollback()
+            await self._obj.rollback()
         elif not self._obj.autocommit:
             await self._obj.commit()
         await self._obj.close()

ci/Dockerfile

@@ -1,4 +1,5 @@
-FROM python:3.6.1-slim
+# NOTE: stretch was very crashy when using https://dev.mysql.com/get/Downloads/Connector-ODBC/8.0/mysql-connector-odbc-8.0.16-linux-debian9-x86-64bit.tar.gz
+FROM python:3.6.8-slim-jessie
 
 # configure apt to install minimal dependencies in non-interactive mode.
 ENV DEBIAN_FRONTEND noninteractive
@@ -23,5 +24,4 @@ RUN rm -rf /aioodbc && \
     apt-get purge -y wget && \
     apt-get autoremove -y
 
-VOLUME /aioodbc
 WORKDIR /aioodbc

requirements-dev.txt

@@ -14,11 +14,13 @@ flake8-pyi==18.3.1
 flake8-tuple==0.2.13
 ipdb==0.11
 ipython==7.2.0
-pyodbc==4.0.25
-pytest==4.0.2
-pytest-asyncio==0.9.0
-pytest-cov==2.6.0
+pyodbc==4.0.26
+pytest==4.6.4
+pytest-asyncio==0.10.0
+pytest-cov==2.7.1
 pytest-sugar==0.9.2
+pytest-faulthandler==1.6.0
 sphinx==1.8.3
 sphinxcontrib-asyncio==0.2.0
-uvloop==0.11.3
+uvloop==0.12.2
+async_generator==1.10