From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Fri, 14 Jul 2017 22:06:48 +0000 (-0400)
Subject: Don't do recovery operations under Empty/AttributeError
X-Git-Tag: origin~41^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f87069a3b0e9c2d79eced81a0326a9a904106dc6;p=thirdparty%2Fsqlalchemy%2Fsqlalchemy.git

Don't do recovery operations under Empty/AttributeError

Made some adjustments to :class:`.Pool` and :class:`.Connection` such
that recovery logic is not run underneath exception catches for
``pool.Empty``, ``AttributeError``, since when the recovery operation
itself fails, Python 3 creates a misleading stack trace referring to the
``Empty`` / ``AttributeError`` as the cause, when in fact these exception
catches are part of control flow.

Change-Id: Id3ed9a8f96ce4ccb4009c94af30ddc2ddb9818b9
Fixes: #4028
---

diff --git a/doc/build/changelog/unreleased_12/4028.rst b/doc/build/changelog/unreleased_12/4028.rst
new file mode 100644
index 0000000000..c6b366569d
--- /dev/null
+++ b/doc/build/changelog/unreleased_12/4028.rst
@@ -0,0 +1,11 @@
+.. change:: 4028
+    :tags: bug, engine
+    :tickets: 4028
+
+    Made some adjustments to :class:`.Pool` and :class:`.Connection` such
+    that recovery logic is not run underneath exception catches for
+    ``pool.Empty``, ``AttributeError``, since when the recovery operation
+    itself fails, Python 3 creates a misleading stack trace referring to the
+    ``Empty`` / ``AttributeError`` as the cause, when in fact these exception
+    catches are part of control flow.
+
diff --git a/lib/sqlalchemy/engine/base.py b/lib/sqlalchemy/engine/base.py
index 91f4493c2e..b5c95cb17b 100644
--- a/lib/sqlalchemy/engine/base.py
+++ b/lib/sqlalchemy/engine/base.py
@@ -345,10 +345,13 @@ class Connection(Connectable):
         try:
             return self.__connection
         except AttributeError:
-            try:
-                return self._revalidate_connection()
-            except BaseException as e:
-                self._handle_dbapi_exception(e, None, None, None, None)
+            # escape "except AttributeError" before revalidating
+            # to prevent misleading stacktraces in Py3K
+            pass
+        try:
+            return self._revalidate_connection()
+        except BaseException as e:
+            self._handle_dbapi_exception(e, None, None, None, None)
 
     def get_isolation_level(self):
         """Return the current isolation level assigned to this
@@ -962,6 +965,10 @@ class Connection(Connectable):
             try:
                 conn = self.__connection
             except AttributeError:
+                # escape "except AttributeError" before revalidating
+                # to prevent misleading stacktraces in Py3K
+                conn = None
+            if conn is None:
                 conn = self._revalidate_connection()
 
             dialect = self.dialect
@@ -1111,6 +1118,10 @@ class Connection(Connectable):
             try:
                 conn = self.__connection
             except AttributeError:
+                # escape "except AttributeError" before revalidating
+                # to prevent misleading stacktraces in Py3K
+                conn = None
+            if conn is None:
                 conn = self._revalidate_connection()
 
             context = constructor(dialect, self, conn, *args)
diff --git a/lib/sqlalchemy/pool.py b/lib/sqlalchemy/pool.py
index a889d1bba2..13e4183906 100644
--- a/lib/sqlalchemy/pool.py
+++ b/lib/sqlalchemy/pool.py
@@ -1164,23 +1164,27 @@ class QueuePool(Pool):
             wait = use_overflow and self._overflow >= self._max_overflow
             return self._pool.get(wait, self._timeout)
         except sqla_queue.Empty:
-            if use_overflow and self._overflow >= self._max_overflow:
-                if not wait:
-                    return self._do_get()
-                else:
-                    raise exc.TimeoutError(
-                        "QueuePool limit of size %d overflow %d reached, "
-                        "connection timed out, timeout %d" %
-                        (self.size(), self.overflow(), self._timeout))
-
-            if self._inc_overflow():
-                try:
-                    return self._create_connection()
-                except:
-                    with util.safe_reraise():
-                        self._dec_overflow()
-            else:
+            # don't do things inside of "except Empty", because when we say
+            # we timed out or can't connect and raise, Python 3 tells
+            # people the real error is queue.Empty which it isn't.
+            pass
+        if use_overflow and self._overflow >= self._max_overflow:
+            if not wait:
                 return self._do_get()
+            else:
+                raise exc.TimeoutError(
+                    "QueuePool limit of size %d overflow %d reached, "
+                    "connection timed out, timeout %d" %
+                    (self.size(), self.overflow(), self._timeout))
+
+        if self._inc_overflow():
+            try:
+                return self._create_connection()
+            except:
+                with util.safe_reraise():
+                    self._dec_overflow()
+        else:
+            return self._do_get()
 
     def _inc_overflow(self):
         if self._max_overflow == -1: