From 0b28b04ebbfaed981f4e00e69e4c7a8e0ed75395 Mon Sep 17 00:00:00 2001
From: Zhao Chao <zhaochao1984@gmail.com>
Date: Tue, 6 Feb 2018 15:31:42 +0800
Subject: [PATCH] Handle error response for webob>=1.6.0

WebOb change https://github.com/Pylons/webob/pull/230 changed
the way in which the error response body is formatted such that
it's no longer a nested dict. So we have to handle both the
old convention of an error message key to the response body error
dict and the new way with just the error body dict.

This was reported upstream:
https://github.com/Pylons/webob/issues/235

But given this was apparently implemented as a long-overdue change
in WebOb the behavior is not likely to change.

Change-Id: Id8bfde6277640de48768bbe5a5e4252b97253ed2
Closes-Bug: #1559072
Co-Authored-By: Matt Riedemann <mriedem@us.ibm.com>
Signed-off-by: Zhao Chao <zhaochao1984@gmail.com>
---
 troveclient/apiclient/exceptions.py           | 24 +++++++--
 .../tests/test_apiclient_exceptions.py        | 51 +++++++++++++++++++
 troveclient/tests/utils.py                    |  4 ++
 3 files changed, 75 insertions(+), 4 deletions(-)
 create mode 100644 troveclient/tests/test_apiclient_exceptions.py

diff --git a/troveclient/apiclient/exceptions.py b/troveclient/apiclient/exceptions.py
index c48df3a6..5c70c08d 100644
--- a/troveclient/apiclient/exceptions.py
+++ b/troveclient/apiclient/exceptions.py
@@ -426,10 +426,26 @@ def from_response(response, method, url):
         except ValueError:
             pass
         else:
-            if isinstance(body, dict):
-                error = list(body.values())[0]
-                kwargs["message"] = error.get("message", None)
-                kwargs["details"] = error.get("details", None)
+            if hasattr(body, 'keys'):
+                # NOTE(mriedem): WebOb<1.6.0 will return a nested dict
+                # structure  where the error keys to the
+                # message/details/code. WebOb>=1.6.0 returns just a response
+                # body as a single dict, not nested, so we have to handle
+                # both cases (since we can't trust what we're given with
+                # content_type: application/json either way.
+                if 'message' in body:
+                    # WebOb 1.6.0 case
+                    message = body.get('message')
+                    details = body.get('details')
+                else:
+                    # WebOb<1.6.0 where we assume there is a single error
+                    # message key to the body that has the message and details.
+                    error = list(body.values())[0]
+                    message = error.get("message")
+                    details = error.get("details")
+
+                kwargs["message"] = message
+                kwargs["details"] = details
     elif content_type.startswith("text/"):
         kwargs["details"] = response.text
 
diff --git a/troveclient/tests/test_apiclient_exceptions.py b/troveclient/tests/test_apiclient_exceptions.py
new file mode 100644
index 00000000..15746b62
--- /dev/null
+++ b/troveclient/tests/test_apiclient_exceptions.py
@@ -0,0 +1,51 @@
+#   Licensed under the Apache License, Version 2.0 (the "License"); you may
+#   not use this file except in compliance with the License. You may obtain
+#   a copy of the License at
+#
+#        http://www.apache.org/licenses/LICENSE-2.0
+#
+#   Unless required by applicable law or agreed to in writing, software
+#   distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#   WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#   License for the specific language governing permissions and limitations
+#   under the License.
+
+import json
+
+from troveclient.apiclient import exceptions
+from troveclient.tests import utils as test_utils
+
+
+class ExceptionsTestCase(test_utils.TestCase):
+
+    def _test_from_response(self, body):
+        data = {
+            'status_code': 503,
+            'headers': {
+                'Content-Type': 'application/json',
+                'x-compute-request-id': (
+                    'req-65d6443c-5910-4eb4-b48a-e69849c26836'),
+            },
+            'text': json.dumps(body)
+        }
+        response = test_utils.TestResponse(data)
+        fake_url = 'http://localhost:8779/v1.0/fake/instances'
+        error = exceptions.from_response(response, 'GET', fake_url)
+        self.assertIsInstance(error, exceptions.ServiceUnavailable)
+
+    def test_from_response_webob_pre_1_6_0(self):
+        # Tests error responses before webob 1.6.0 where the error details
+        # are nested in the response body.
+        body = {
+            'serviceUnavailable': {
+                'message': 'Fake message.',
+                'code': 503
+            }
+        }
+        self._test_from_response(body)
+
+    def test_from_response_webob_post_1_6_0(self):
+        # Tests error responses from webob 1.6.0 where the error details
+        # are in the response body.
+        body = {'message': 'Fake message.', 'code': 503}
+        self._test_from_response(body)
diff --git a/troveclient/tests/utils.py b/troveclient/tests/utils.py
index a912649a..77d85064 100644
--- a/troveclient/tests/utils.py
+++ b/troveclient/tests/utils.py
@@ -95,3 +95,7 @@ class TestResponse(requests.Response):
             self._text = data.get('text')
         else:
             self.status_code = data
+
+    @property
+    def text(self):
+        return self._text