Repository: globus/globus-sdk-python
Pull request #155 (2017-03-08 16:54:02)

Improve requests error wrapping
Addresses #152 

I gave all the exceptions very simple messages for now. We could expand on the conversion logic to look at the underlying exceptions and give more helpful messages based on those data if wanted.


Patch:
diff --git a/globus_sdk/base.py b/globus_sdk/base.py
index 3bc0befb..d1a1b3b6 100644
--- a/globus_sdk/base.py
+++ b/globus_sdk/base.py
@@ -329,15 +329,9 @@ class BaseClient(object):
                 return self._session.request(
                     method=method, url=url, headers=rheaders, params=params,
                     data=text_body, verify=self._verify)
-            except requests.Timeout as e:
-                self.logger.error("TimeoutError on request")
-                raise exc.GlobusTimeoutError(*e.args)
-            except requests.ConnectionError as e:
-                self.logger.error("ConnectionError on request")
-                raise exc.GlobusConnectionError(*e.args)
             except requests.RequestException as e:
                 self.logger.error("NetworkError on request")
-                raise exc.NetworkError(*e.args)
+                raise exc.convert_request_exception(e)
 
         # initial request
         r = send_request()
diff --git a/globus_sdk/exc.py b/globus_sdk/exc.py
index f0005d56..ae09dc20 100644
--- a/globus_sdk/exc.py
+++ b/globus_sdk/exc.py
@@ -1,5 +1,6 @@
 import logging
 import textwrap
+import requests
 
 logger = logging.getLogger(__name__)
 
@@ -141,11 +142,17 @@ class InvalidDocumentBodyError(GlobusError):
     """
 
 
-# Thin wrappers around requests exceptions, so the SDK is API independent.
+# Wrappers around requests exceptions, so the SDK is API independent.
 class NetworkError(GlobusError):
     """
     Error communicating with the REST API server.
+
+    Holds onto original exception data, but also takes a message
+    to explain potentially confusing or inconsistent exceptions passed to us
     """
+    def __init__(self, msg, exc, *args, **kw):
+        super(NetworkError, self).__init__(msg)
+        self.underlying_exception = exc
 
 
 class GlobusTimeoutError(NetworkError):
@@ -156,6 +163,17 @@ class GlobusConnectionError(NetworkError):
     """A connection error occured while making a REST request."""
 
 
+def convert_request_exception(exc):
+    """Converts incoming requests.Exception to a Globus NetworkError"""
+
+    if isinstance(exc, requests.Timeout):
+        return GlobusTimeoutError("TimeoutError on request", exc)
+    elif isinstance(exc, requests.ConnectionError):
+        return GlobusConnectionError("ConnectionError on request", exc)
+    else:
+        return NetworkError("NetworkError on request", exc)
+
+
 class GlobusOptionalDependencyError(GlobusError, NotImplementedError):
     """
     Error class for attempts to use features only enabled via optional


Test patch:
diff --git a/tests/unit/test_exc.py b/tests/unit/test_exc.py
index eb448456..638259db 100644
--- a/tests/unit/test_exc.py
+++ b/tests/unit/test_exc.py
@@ -3,7 +3,9 @@ import json
 import six
 
 from globus_sdk.exc import (GlobusAPIError, TransferAPIError,
-                            GlobusOptionalDependencyError)
+                            GlobusOptionalDependencyError,
+                            NetworkError, GlobusTimeoutError,
+                            GlobusConnectionError, convert_request_exception)
 from tests.framework import CapturedIOTestCase
 
 
@@ -149,6 +151,78 @@ class TransferAPIErrorTests(APIErrorTests):
         self.assertEqual(apiErr.exception._get_args(), expected)
 
 
+class NetworkErrorTests(CapturedIOTestCase):
+
+    def setUp(self):
+        """
+        Sets up simple exceptions to wrap
+        """
+        super(NetworkErrorTests, self).setUp()
+        self.exc = requests.RequestException("exc_message")
+        self.timeout_exc = requests.Timeout("timeout_message")
+        self.connection_exc = requests.ConnectionError("connection_message")
+
+    def test_networkError(self):
+        """
+        Raises a NetworkError wrapped around a RequestException
+        Confirms wrapped data is available, but new message is used
+        """
+        msg = "wrapper message"
+        with self.assertRaises(NetworkError) as err:
+            raise NetworkError(msg, self.exc)
+
+        self.assertEqual(str(err.exception), msg)
+        self.assertEqual(err.exception.underlying_exception.args,
+                         self.exc.args)
+
+    def test_timeoutError(self):
+        """
+        Raises a GlobusTimeoutError wrapped around a request Timeout
+        Confirms wrapped data is available, but new message is used
+        """
+        msg = "wrapper message"
+        with self.assertRaises(GlobusTimeoutError) as err:
+            raise GlobusTimeoutError(msg, self.timeout_exc)
+
+        self.assertEqual(str(err.exception), msg)
+        self.assertEqual(err.exception.underlying_exception.args,
+                         self.timeout_exc.args)
+
+    def test_connectionError(self):
+        """
+        Raises a GlobusTimeoutError wrapped around a request ConnectionError
+        Confirms wrapped data is available, but new message is used
+        """
+        msg = "wrapper message"
+        with self.assertRaises(GlobusConnectionError) as err:
+            raise GlobusConnectionError(msg, self.connection_exc)
+
+        self.assertEqual(str(err.exception), msg)
+        self.assertEqual(err.exception.underlying_exception.args,
+                         self.connection_exc.args)
+
+    def test_convert_request_exception(self):
+        """
+        Converts known request exceptions into Globus NetworkErrors,
+        confirms expected values.
+        """
+        # NetworkError
+        conv = convert_request_exception(self.exc)
+        self.assertIsInstance(conv, NetworkError)
+        self.assertEqual(conv.underlying_exception.args,
+                         self.exc.args)
+        # Timeout Error
+        conv = convert_request_exception(self.timeout_exc)
+        self.assertIsInstance(conv, GlobusTimeoutError)
+        self.assertEqual(conv.underlying_exception.args,
+                         self.timeout_exc.args)
+        # Connection Error
+        conv = convert_request_exception(self.connection_exc)
+        self.assertIsInstance(conv, GlobusConnectionError)
+        self.assertEqual(conv.underlying_exception.args,
+                         self.connection_exc.args)
+
+
 class GlobusOptionalDependencyErrorTests(CapturedIOTestCase):
 
     def test_init(self):