Proper errors handling for client

* Introduce a specific exception class (subclasses requests.HTTPError
  to be backward compatible)
* Fetch error message from response body
* i18nize existing errors

Change-Id: I71922895c3177720789c6933da79b8882bfa7127
Closes-Bug: #1428680

README.rst

@@ -364,6 +364,10 @@ See `1.1.0 release tracking page`_ for details.
 
 * Proper CLI tool implemented as a plugin for OpenStackClient_.
 
+  Also client now properly sets error message from the server in its exception.
+  This might be a breaking change, if you relied on exception message
+  previously.
+
 * The default value for ``overwrite_existing`` configuration option was
   flipped, matching the default behavior for Ironic inspection.
 

ironic_discoverd/client.py

@@ -19,8 +19,11 @@ import json
 import requests
 import six
 
+from ironic_discoverd.common.i18n import _
+
 
 _DEFAULT_URL = 'http://127.0.0.1:5050/v1'
+_ERROR_ENCODING = 'utf-8'
 
 
 def _prepare(base_url, auth_token):
@@ -31,6 +34,20 @@ def _prepare(base_url, auth_token):
     return base_url, headers
 
 
+class ClientError(requests.HTTPError):
+    """Error returned from a server."""
+    def __init__(self, response):
+        # discoverd returns error message in body
+        msg = response.content.decode(_ERROR_ENCODING)
+        super(ClientError, self).__init__(msg, response=response)
+
+    @classmethod
+    def raise_if_needed(cls, response):
+        """Raise exception if response contains error."""
+        if response.status_code >= 400:
+            raise cls(response)
+
+
 def introspect(uuid, base_url=None, auth_token=None,
                new_ipmi_password=None, new_ipmi_username=None):
     """Start introspection for a node.
@@ -46,16 +63,16 @@ def introspect(uuid, base_url=None, auth_token=None,
                               driver_info.
     """
     if not isinstance(uuid, six.string_types):
-        raise TypeError("Expected string for uuid argument, got %r" % uuid)
+        raise TypeError(_("Expected string for uuid argument, got %r") % uuid)
     if new_ipmi_username and not new_ipmi_password:
-        raise ValueError("Setting IPMI user name requires a new password")
+        raise ValueError(_("Setting IPMI user name requires a new password"))
 
     base_url, headers = _prepare(base_url, auth_token)
     params = {'new_ipmi_username': new_ipmi_username,
               'new_ipmi_password': new_ipmi_password}
     res = requests.post("%s/introspection/%s" % (base_url, uuid),
                         headers=headers, params=params)
-    res.raise_for_status()
+    ClientError.raise_if_needed(res)
 
 
 def get_status(uuid, base_url=None, auth_token=None):
@@ -69,12 +86,12 @@ def get_status(uuid, base_url=None, auth_token=None):
     :raises: *requests* library HTTP errors.
     """
     if not isinstance(uuid, six.string_types):
-        raise TypeError("Expected string for uuid argument, got %r" % uuid)
+        raise TypeError(_("Expected string for uuid argument, got %r") % uuid)
 
     base_url, headers = _prepare(base_url, auth_token)
     res = requests.get("%s/introspection/%s" % (base_url, uuid),
                        headers=headers)
-    res.raise_for_status()
+    ClientError.raise_if_needed(res)
     return res.json()
 
 
@@ -84,14 +101,14 @@ def discover(uuids, base_url=None, auth_token=None):
     DEPRECATED. Use introspect instead.
     """
     if not all(isinstance(s, six.string_types) for s in uuids):
-        raise TypeError("Expected list of strings for uuids argument, got %s" %
-                        uuids)
+        raise TypeError(_("Expected list of strings for uuids argument, "
+                          "got %s") % uuids)
 
     base_url, headers = _prepare(base_url, auth_token)
     headers['Content-Type'] = 'application/json'
     res = requests.post(base_url + "/discover",
                         data=json.dumps(uuids), headers=headers)
-    res.raise_for_status()
+    ClientError.raise_if_needed(res)
 
 
 if __name__ == '__main__':  # pragma: no cover

ironic_discoverd/test/test_client.py

@@ -19,7 +19,8 @@ from oslo_utils import uuidutils
 from ironic_discoverd import client
 
 
-@mock.patch.object(client.requests, 'post', autospec=True)
+@mock.patch.object(client.requests, 'post', autospec=True,
+                   **{'return_value.status_code': 200})
 class TestIntrospect(unittest.TestCase):
     def setUp(self):
         super(TestIntrospect, self).setUp()
@@ -74,8 +75,15 @@ class TestIntrospect(unittest.TestCase):
             params={'new_ipmi_username': None, 'new_ipmi_password': None}
         )
 
+    def test_failed(self, mock_post):
+        mock_post.return_value.status_code = 404
+        mock_post.return_value.content = b"boom"
+        self.assertRaisesRegexp(client.ClientError, "boom",
+                                client.introspect, self.uuid)
 
-@mock.patch.object(client.requests, 'post', autospec=True)
+
+@mock.patch.object(client.requests, 'post', autospec=True,
+                   **{'return_value.status_code': 200})
 class TestDiscover(unittest.TestCase):
     def setUp(self):
         super(TestDiscover, self).setUp()
@@ -97,8 +105,15 @@ class TestDiscover(unittest.TestCase):
         self.assertRaises(TypeError, client.discover, 42)
         self.assertRaises(TypeError, client.discover, [42])
 
+    def test_failed(self, mock_post):
+        mock_post.return_value.status_code = 404
+        mock_post.return_value.content = b"boom"
+        self.assertRaisesRegexp(client.ClientError, "boom",
+                                client.discover, [self.uuid])
+
 
-@mock.patch.object(client.requests, 'get', autospec=True)
+@mock.patch.object(client.requests, 'get', autospec=True,
+                   **{'return_value.status_code': 200})
 class TestGetStatus(unittest.TestCase):
     def setUp(self):
         super(TestGetStatus, self).setUp()
@@ -116,3 +131,9 @@ class TestGetStatus(unittest.TestCase):
 
     def test_invalid_input(self, _):
         self.assertRaises(TypeError, client.get_status, 42)
+
+    def test_failed(self, mock_post):
+        mock_post.return_value.status_code = 404
+        mock_post.return_value.content = b"boom"
+        self.assertRaisesRegexp(client.ClientError, "boom",
+                                client.get_status, self.uuid)