Placement client: improve Placement 4xx exceptions

For 4xx errors Placement sends back a complex JSON object describing
the error. When turned into an exception that becomes a non-trivial
attribute of the error object. Usual ways of logging an exception (that
is LOG.exception) completely ignore that attribute, therefore the real
error message is not logged. For example we only logged the fact that
we received a BadRequest response and nothing else while Placement did
provide a whole lot more detail. Here we dig out that error detail and
re-throw a better exception with it.

Change-Id: Id97116c1c298f54f898a746d6e3c96b1f412bb49
Related-Bug: #1578989

neutron_lib/exceptions/placement.py

@@ -65,3 +65,7 @@ class PlacementAPIVersionIncorrect(exceptions.NotFound):
 class PlacementResourceProviderNameNotUnique(exceptions.Conflict):
     message = _("Another resource provider exists with the provided name: "
                 "%(name)s.")
+
+
+class PlacementClientError(exceptions.NeutronException):
+    message = _("Placement Client Error (4xx): %(msg)s")

neutron_lib/placement/client.py

@@ -43,9 +43,21 @@ def _check_placement_api_available(f):
     """
     @functools.wraps(f)
     def wrapper(self, *a, **k):
-        if not self._client:
-            self._client = self._create_client()
-        return f(self, *a, **k)
+        try:
+            if not self._client:
+                self._client = self._create_client()
+            return f(self, *a, **k)
+        except ks_exc.http.HttpError as exc:
+            if 400 <= exc.http_status <= 499:
+                # NOTE(bence romsics): Placement has inconsistently formatted
+                # error messages. Some error response bodies are JSON
+                # formatted, seemingly machine readible objects. While others
+                # are free format text. We have to keep the whole thing
+                # to avoid losing information.
+                raise n_exc.PlacementClientError(
+                    msg=exc.response.text.replace('\n', ' '))
+            else:
+                raise
     return wrapper
 
 

neutron_lib/tests/unit/placement/test_client.py

@@ -223,8 +223,9 @@ class TestPlacementAPIClient(base.BaseTestCase):
     def test_get_inventory_not_found(self):
         _exception = ks_exc.NotFound()
         _exception.details = "Any other exception explanation"
+        _exception.response = mock.Mock(text="Some error response body")
         self.placement_fixture.mock_get.side_effect = _exception
-        self.assertRaises(ks_exc.NotFound,
+        self.assertRaises(n_exc.PlacementClientError,
                           self.placement_api_client.get_inventory,
                           RESOURCE_PROVIDER_UUID, RESOURCE_CLASS_NAME)
 

releasenotes/notes/placement-client-do-not-swallow-exceptions-c33c9a9224a27551.yaml

@@ -0,0 +1,8 @@
+---
+other:
+  - |
+    The Placement client previously swallowed a few exceptions (but logged
+    a warning when doing this). In order to let the user of the client choose
+    to handle or ignore the error condition the client no longer does this.
+    Also to avoid losing error information we catch and re-throw HTTP 4xx
+    exceptions with better messages.