Support bare metal service error messages

The bare metal service returns errors in a non-standard format. To make it
worse, it also JSON-encodes errors twice due to weird interaction between
WSME and Pecan. This patch implements both the double-encoded and the future
correct format.

Change-Id: Icfdd223e3a2b6f7b390be8d6581007be8b14666f

openstack/exceptions.py

@@ -16,6 +16,7 @@
 Exception definitions.
 """
 
+import json
 import re
 
 from requests import exceptions as _rex
@@ -160,6 +161,24 @@ class InvalidResourceQuery(SDKException):
     pass
 
 
+def _extract_message(obj):
+    if isinstance(obj, dict):
+        # Most of services: compute, network
+        if obj.get('message'):
+            return obj['message']
+        # Ironic starting with Stein
+        elif obj.get('faultstring'):
+            return obj['faultstring']
+    elif isinstance(obj, six.string_types):
+        # Ironic before Stein has double JSON encoding, nobody remembers why.
+        try:
+            obj = json.loads(obj)
+        except Exception:
+            pass
+        else:
+            return _extract_message(obj)
+
+
 def raise_from_response(response, error_message=None):
     """Raise an instance of an HTTPException based on keystoneauth response."""
     if response.status_code < 400:
@@ -183,8 +202,7 @@ def raise_from_response(response, error_message=None):
 
         try:
             content = response.json()
-            messages = [obj.get('message') for obj in content.values()
-                        if isinstance(obj, dict)]
+            messages = [_extract_message(obj) for obj in content.values()]
             # Join all of the messages together nicely and filter out any
             # objects that don't have a "message" attr.
             details = '\n'.join(msg for msg in messages if msg)
@@ -197,10 +215,9 @@ def raise_from_response(response, error_message=None):
         details = list(set([msg for msg in details if msg]))
         # Return joined string separated by colons.
         details = ': '.join(details)
-    if not details and response.reason:
-        details = response.reason
-    else:
-        details = response.text
+
+    if not details:
+        details = response.reason if response.reason else response.text
 
     http_status = response.status_code
     request_id = response.headers.get('x-openstack-request-id')

openstack/tests/unit/test_exceptions.py

@@ -10,6 +10,8 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import json
+
 import mock
 from openstack.tests.unit import base
 import uuid
@@ -123,3 +125,82 @@ class TestRaiseFromResponse(base.TestCase):
             response.headers.get('x-openstack-request-id'),
             exc.request_id
         )
+
+    def test_raise_compute_format(self):
+        response = mock.Mock()
+        response.status_code = 404
+        response.headers = {
+            'content-type': 'application/json',
+        }
+        response.json.return_value = {
+            'itemNotFound': {
+                'message': self.message,
+                'code': 404,
+            }
+        }
+        exc = self.assertRaises(exceptions.NotFoundException,
+                                self._do_raise, response,
+                                error_message=self.message)
+        self.assertEqual(response.status_code, exc.status_code)
+        self.assertEqual(self.message, exc.details)
+        self.assertIn(self.message, str(exc))
+
+    def test_raise_network_format(self):
+        response = mock.Mock()
+        response.status_code = 404
+        response.headers = {
+            'content-type': 'application/json',
+        }
+        response.json.return_value = {
+            'NeutronError': {
+                'message': self.message,
+                'type': 'FooNotFound',
+                'detail': '',
+            }
+        }
+        exc = self.assertRaises(exceptions.NotFoundException,
+                                self._do_raise, response,
+                                error_message=self.message)
+        self.assertEqual(response.status_code, exc.status_code)
+        self.assertEqual(self.message, exc.details)
+        self.assertIn(self.message, str(exc))
+
+    def test_raise_baremetal_old_format(self):
+        response = mock.Mock()
+        response.status_code = 404
+        response.headers = {
+            'content-type': 'application/json',
+        }
+        response.json.return_value = {
+            'error_message': json.dumps({
+                'faultstring': self.message,
+                'faultcode': 'Client',
+                'debuginfo': None,
+            })
+        }
+        exc = self.assertRaises(exceptions.NotFoundException,
+                                self._do_raise, response,
+                                error_message=self.message)
+        self.assertEqual(response.status_code, exc.status_code)
+        self.assertEqual(self.message, exc.details)
+        self.assertIn(self.message, str(exc))
+
+    def test_raise_baremetal_corrected_format(self):
+        response = mock.Mock()
+        response.status_code = 404
+        response.headers = {
+            'content-type': 'application/json',
+        }
+        response.json.return_value = {
+            'error_message': {
+                'faultstring': self.message,
+                'faultcode': 'Client',
+                'debuginfo': None,
+            }
+        }
+        exc = self.assertRaises(exceptions.NotFoundException,
+                                self._do_raise, response,
+                                error_message=self.message)
+        self.assertEqual(response.status_code, exc.status_code)
+        self.assertEqual(self.message, exc.details)
+        self.assertIn(self.message, str(exc))

releasenotes/notes/baremetal-errors-5cc871e8df4c9d95.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    Adds support for error messages from the bare metal service.