From 01d2da9e4776fed0f2551a4c6e39a72865d5ca72 Mon Sep 17 00:00:00 2001 From: Michael McCune Date: Thu, 30 May 2019 14:17:42 -0400 Subject: [PATCH] add handling for multiple error returns This change adds logic to the `exceptions.from_response` to handle errors formatted in accordance with the API-SIG guidelines. When there are multiple errors returned, only the first error will be included in the exception with a note informing that there were more errors. API SIG guideline: https://specs.openstack.org/openstack/api-sig/guidelines/errors.html email thread for content: http://lists.openstack.org/pipermail/openstack-discuss/2019-May/006377.html related neutron bug: https://bugs.launchpad.net/neutron/+bug/1828543 Change-Id: I1f06c2cd5c4e93e04582d4ffbb434db92010d712 --- keystoneauth1/exceptions/http.py | 24 +++++ keystoneauth1/tests/unit/test_session.py | 99 +++++++++++++++++++ .../api-sig-error-guideline-handler.yaml | 4 + 3 files changed, 127 insertions(+) create mode 100644 releasenotes/notes/api-sig-error-guideline-handler.yaml diff --git a/keystoneauth1/exceptions/http.py b/keystoneauth1/exceptions/http.py index f76afc24..768442e5 100644 --- a/keystoneauth1/exceptions/http.py +++ b/keystoneauth1/exceptions/http.py @@ -413,6 +413,30 @@ def from_response(response, method, url): error = body["error"] kwargs["message"] = error.get("message") kwargs["details"] = error.get("details") + elif (isinstance(body, dict) and + isinstance(body.get("errors"), list)): + # if the error response follows the API SIG guidelines, it + # will return a list of errors. in this case, only the first + # error is shown, but if there are multiple the user will be + # alerted to that fact. + errors = body["errors"] + if len(errors) == 0: + # just in case we get an empty array + kwargs["message"] = None + kwargs["details"] = None + else: + if len(errors) > 1: + # if there is more than one error, let the user know + # that multiple were seen. + msg_hdr = ("Multiple error responses, " + "showing first only: ") + else: + msg_hdr = "" + + kwargs["message"] = "{}{}".format(msg_hdr, + errors[0].get("title")) + kwargs["details"] = errors[0].get("detail") + elif content_type.startswith("text/"): kwargs["details"] = response.text diff --git a/keystoneauth1/tests/unit/test_session.py b/keystoneauth1/tests/unit/test_session.py index 7dc8e55b..0a39123e 100644 --- a/keystoneauth1/tests/unit/test_session.py +++ b/keystoneauth1/tests/unit/test_session.py @@ -542,6 +542,105 @@ class SessionTests(utils.TestCase): 'value': 'new_name'}]) self.assertContentTypeIs('application/json-patch+json') + def test_api_sig_error_message_single(self): + title = 'this error is bogus!' + detail = 'it is a totally made up error' + error_message = { + 'errors': [ + { + 'request_id': uuid.uuid4().hex, + 'code': 'phoney.bologna.error', + 'status': 500, + 'title': title, + 'detail': detail, + 'links': [ + { + 'rel': 'help', + 'href': 'https://openstack.org' + } + ] + } + ] + } + payload = json.dumps(error_message) + self.stub_url('GET', status_code=9000, text=payload, + headers={'Content-Type': 'application/json'}) + session = client_session.Session() + + # The exception should contain the information from the error response + msg = '{} (HTTP 9000)'.format(title) + try: + session.get(self.TEST_URL) + except exceptions.HttpError as ex: + self.assertEqual(ex.message, msg) + self.assertEqual(ex.details, detail) + + def test_api_sig_error_message_multiple(self): + title = 'this error is the first error!' + detail = 'it is a totally made up error' + error_message = { + 'errors': [ + { + 'request_id': uuid.uuid4().hex, + 'code': 'phoney.bologna.error', + 'status': 500, + 'title': title, + 'detail': detail, + 'links': [ + { + 'rel': 'help', + 'href': 'https://openstack.org' + } + ] + }, + { + 'request_id': uuid.uuid4().hex, + 'code': 'phoney.bologna.error', + 'status': 500, + 'title': 'some other error', + 'detail': detail, + 'links': [ + { + 'rel': 'help', + 'href': 'https://openstack.org' + } + ] + } + ] + } + payload = json.dumps(error_message) + self.stub_url('GET', status_code=9000, text=payload, + headers={'Content-Type': 'application/json'}) + session = client_session.Session() + + # The exception should contain the information from the error response + msg = ('Multiple error responses, showing first only: {} (HTTP 9000)' + .format(title)) + try: + session.get(self.TEST_URL) + except exceptions.HttpError as ex: + self.assertEqual(ex.message, msg) + self.assertEqual(ex.details, detail) + + def test_api_sig_error_message_empty(self): + error_message = { + 'errors': [ + ] + } + payload = json.dumps(error_message) + self.stub_url('GET', status_code=9000, text=payload, + headers={'Content-Type': 'application/json'}) + session = client_session.Session() + + # The exception should contain the information from the error response + msg = 'HTTP Error (HTTP 9000)' + + try: + session.get(self.TEST_URL) + except exceptions.HttpError as ex: + self.assertEqual(ex.message, msg) + self.assertIsNone(ex.details) + class RedirectTests(utils.TestCase): diff --git a/releasenotes/notes/api-sig-error-guideline-handler.yaml b/releasenotes/notes/api-sig-error-guideline-handler.yaml new file mode 100644 index 00000000..ebfa7f8a --- /dev/null +++ b/releasenotes/notes/api-sig-error-guideline-handler.yaml @@ -0,0 +1,4 @@ +--- +features: + - Fix handling of HTTP error payloads that conform to the API SIG + formatting guidelines.