Do not expose exceptions from requests library

Novaclient expects the requests library to always be able to get a
response back and doesn't handle other cases where it might raise
an exception even before the request is completed (e.g.
ConnectionError).

These exceptions should be captured and properly converted to some
native novaclient exception.

Change-Id: Iec7247b715c38c29910b30e76413cead4e951a37
Co-Authored-By: Kuo-tung Kao <jelly.k@inwinstack.com>
Closes-Bug: #1406586

novaclient/client.py

@@ -84,13 +84,12 @@ class SessionClient(adapter.LegacyJsonAdapter):
         # NOTE(jamielennox): The standard call raises errors from
         # keystoneclient, where we need to raise the novaclient errors.
         raise_exc = kwargs.pop('raise_exc', True)
-        with utils.record_time(self.times, self.timings, method, url):
-            resp, body = super(SessionClient, self).request(url,
-                                                            method,
-                                                            raise_exc=False,
-                                                            **kwargs)
-        if raise_exc and resp.status_code >= 400:
-            raise exceptions.from_response(resp, body, url, method)
+        with utils.converted_exceptions():
+            with utils.record_time(self.times, self.timings, method, url):
+                resp, body = super(SessionClient, self).request(
+                    url, method, raise_exc=False, **kwargs)
+            if raise_exc and resp.status_code >= 400:
+                raise exceptions.from_response(resp, body, url, method)
 
         return resp, body
 
@@ -360,10 +359,11 @@ class HTTPClient(object):
         if session:
             request_func = session.request
 
-        resp = request_func(
-            method,
-            url,
-            **kwargs)
+        with utils.converted_exceptions():
+            resp = request_func(
+                method,
+                url,
+                **kwargs)
 
         self.http_log_resp(resp)
 

novaclient/exceptions.py

@@ -283,3 +283,26 @@ def from_response(response, body, url, method=None):
 class ResourceNotFound(Exception):
     """Error in getting the resource."""
     pass
+
+
+class RequestTimeout(ClientException):
+    """
+    HTTP 408 - Request Timeout
+    """
+    http_status = 408
+    message = "Request Timeout"
+
+
+class TooManyRedirects(Exception):
+    """Too many redirects."""
+    pass
+
+
+class ConnectionError(Exception):
+    """A Connection error occurred."""
+    pass
+
+
+class RequestException(Exception):
+    """An ambiguous exception occurred."""
+    pass

novaclient/tests/unit/test_client.py

@@ -21,6 +21,7 @@ import fixtures
 from keystoneclient import adapter
 import mock
 import requests
+import six
 
 import novaclient.client
 import novaclient.extension
@@ -438,6 +439,35 @@ class ClientTest(utils.TestCase):
         self.assertEqual(1, len(client.times))
         self.assertEqual('GET http://no.where', client.times[0][0])
 
+    @mock.patch.object(requests, 'request')
+    def test_request_exception_conversions(self, m_request):
+        client = novaclient.client.HTTPClient(user='zqfan', password='')
+
+        exc = requests.HTTPError()
+        exc.response = requests.Response()
+        exc.response.status_code = 12345
+        m_request.side_effect = exc
+        exc = self.assertRaises(novaclient.exceptions.ClientException,
+                                client.request, "http://www.openstack.org",
+                                'GET')
+        self.assertIn("HTTP 12345", six.text_type(exc))
+
+        m_request.side_effect = requests.ConnectionError()
+        self.assertRaises(novaclient.exceptions.ConnectionError,
+                          client.request, "http://www.openstack.org", 'GET')
+
+        m_request.side_effect = requests.Timeout()
+        self.assertRaises(novaclient.exceptions.RequestTimeout,
+                          client.request, "http://www.openstack.org", 'GET')
+
+        m_request.side_effect = requests.TooManyRedirects()
+        self.assertRaises(novaclient.exceptions.TooManyRedirects,
+                          client.request, "http://www.openstack.org", 'GET')
+
+        m_request.side_effect = requests.RequestException()
+        self.assertRaises(novaclient.exceptions.RequestException,
+                          client.request, "http://www.openstack.org", 'GET')
+
 
 class SessionClientTest(utils.TestCase):
 
@@ -454,3 +484,32 @@ class SessionClientTest(utils.TestCase):
         client.request("http://no.where", 'GET')
         self.assertEqual(1, len(client.times))
         self.assertEqual('GET http://no.where', client.times[0][0])
+
+    @mock.patch.object(adapter.LegacyJsonAdapter, 'request')
+    def test_request_exception_conversions(self, m_request):
+        client = novaclient.client.SessionClient(session=mock.MagicMock())
+
+        exc = requests.HTTPError()
+        exc.response = requests.Response()
+        exc.response.status_code = 12345
+        m_request.side_effect = exc
+        exc = self.assertRaises(novaclient.exceptions.ClientException,
+                                client.request, "http://www.openstack.org",
+                                'GET')
+        self.assertIn("HTTP 12345", six.text_type(exc))
+
+        m_request.side_effect = requests.ConnectionError()
+        self.assertRaises(novaclient.exceptions.ConnectionError,
+                          client.request, "http://www.openstack.org", 'GET')
+
+        m_request.side_effect = requests.Timeout()
+        self.assertRaises(novaclient.exceptions.RequestTimeout,
+                          client.request, "http://www.openstack.org", 'GET')
+
+        m_request.side_effect = requests.TooManyRedirects()
+        self.assertRaises(novaclient.exceptions.TooManyRedirects,
+                          client.request, "http://www.openstack.org", 'GET')
+
+        m_request.side_effect = requests.RequestException()
+        self.assertRaises(novaclient.exceptions.RequestException,
+                          client.request, "http://www.openstack.org", 'GET')

novaclient/utils.py

@@ -22,6 +22,7 @@ from oslo_serialization import jsonutils
 from oslo_utils import encodeutils
 import pkg_resources
 import prettytable
+import requests
 import six
 
 from novaclient import exceptions
@@ -376,6 +377,29 @@ def record_time(times, enabled, *args):
         times.append((' '.join(args), start, end))
 
 
+@contextlib.contextmanager
+def converted_exceptions():
+    try:
+        yield
+
+    except requests.HTTPError as e:
+        status_code = e.response.status_code
+        raise exceptions.ClientException(status_code,
+                                         encodeutils.exception_to_unicode(e))
+
+    except requests.ConnectionError as e:
+        raise exceptions.ConnectionError(encodeutils.exception_to_unicode(e))
+
+    except requests.Timeout as e:
+        raise exceptions.RequestTimeout(encodeutils.exception_to_unicode(e))
+
+    except requests.TooManyRedirects as e:
+        raise exceptions.TooManyRedirects(encodeutils.exception_to_unicode(e))
+
+    except requests.RequestException as e:
+        raise exceptions.RequestException(encodeutils.exception_to_unicode(e))
+
+
 def get_function_name(func):
     if six.PY2:
         if hasattr(func, "im_class"):