From 6c112f1dbae41aaf6a62415224709e2c51b643df Mon Sep 17 00:00:00 2001 From: Tao Zou Date: Fri, 27 May 2022 14:40:23 +0800 Subject: [PATCH] Handle response is not a json format Change-Id: I62a6c6d248301576176be5631a4ef4900460082d (cherry picked from commit d915f2fecbc0e629b67873d4923c4aba9f0bcdcc) --- vmware_nsxlib/tests/unit/v3/mocks.py | 10 ++++ .../tests/unit/v3/nsxlib_testcase.py | 2 +- vmware_nsxlib/tests/unit/v3/test_client.py | 7 ++- vmware_nsxlib/tests/unit/v3/test_cluster.py | 19 ++++++ vmware_nsxlib/v3/client.py | 58 +++++++++++++++++++ vmware_nsxlib/v3/cluster.py | 5 +- 6 files changed, 98 insertions(+), 3 deletions(-) diff --git a/vmware_nsxlib/tests/unit/v3/mocks.py b/vmware_nsxlib/tests/unit/v3/mocks.py index d8b07b6e..09a7519b 100644 --- a/vmware_nsxlib/tests/unit/v3/mocks.py +++ b/vmware_nsxlib/tests/unit/v3/mocks.py @@ -90,6 +90,16 @@ class MockRequestsResponse(object): def json(self): return jsonutils.loads(self.content) + def __bool__(self): + """Returns True if :attr:`status_code` is less than 400. + + This attribute checks if the status code of the response is between + 400 and 600 to see if there was a client error or a server error. If + the status code, is between 200 and 400, this will return True. This + is **not** a check to see if the response code is ``200 OK``. + """ + return self.status_code < 400 + class MockRequestSessionApi(object): diff --git a/vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py b/vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py index 546e2b70..6104b19e 100644 --- a/vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py +++ b/vmware_nsxlib/tests/unit/v3/nsxlib_testcase.py @@ -283,7 +283,7 @@ class NsxClientTestCase(NsxLibTestCase): super(NsxClientTestCase.MockHTTPProvider, self).__init__() if isinstance(session_response, list): self._session_responses = session_response - elif session_response: + elif session_response is not None: self._session_responses = [session_response] else: self._session_responses = None diff --git a/vmware_nsxlib/tests/unit/v3/test_client.py b/vmware_nsxlib/tests/unit/v3/test_client.py index 3047d6d7..2384f0ea 100644 --- a/vmware_nsxlib/tests/unit/v3/test_client.py +++ b/vmware_nsxlib/tests/unit/v3/test_client.py @@ -259,7 +259,8 @@ class NsxV3RESTClientTestCase(nsxlib_testcase.NsxClientTestCase): 'error_message': 'bad', 'related_errors': [{ 'error_message': 'bla', - 'error_code': 'code'}]}) + 'error_code': 'code', + 'httpStatus': 'BAD_REQUEST'}]}) response = mocks.MockRequestsResponse( status_code, content) @@ -325,6 +326,10 @@ class NsxV3RESTClientTestCase(nsxlib_testcase.NsxClientTestCase): 500157, [777, 500045]) self.assertEqual(exc, nsxlib_exc.NsxPendingDelete) + code = requests.codes.SERVICE_UNAVAILABLE + exc = client.http_error_to_exception(code, None) + self.assertEqual(exc, nsxlib_exc.ServiceUnavailable) + class NsxV3JSONClientTestCase(nsxlib_testcase.NsxClientTestCase): diff --git a/vmware_nsxlib/tests/unit/v3/test_cluster.py b/vmware_nsxlib/tests/unit/v3/test_cluster.py index c99d7976..d80108fc 100644 --- a/vmware_nsxlib/tests/unit/v3/test_cluster.py +++ b/vmware_nsxlib/tests/unit/v3/test_cluster.py @@ -16,6 +16,7 @@ import unittest import mock +from requests import codes from requests import exceptions as requests_exceptions from requests import models import six.moves.urllib.parse as urlparse @@ -288,6 +289,24 @@ class ClusteredAPITestCase(nsxlib_testcase.NsxClientTestCase): self.assertRaises(nsxlib_exc.StaleRevision, api.get, 'api/v1/transport-zones') + def test_max_retry_attempts_on_none_json_error(self): + # handle response not json format + def server1_error(): + return mocks.MockRequestsResponse( + codes.SERVICE_UNAVAILABLE, + "python-requests/2.27.1 d50561a7-7540-4342-88e3-c235e587a19") + + conf_managers = ['8.9.10.11', '9.10.11.12', '10.11.12.13'] + max_attempts = 2 + api = self.mock_nsx_clustered_api( + nsx_api_managers=conf_managers, + max_attempts=max_attempts, + session_response=[server1_error for i in range(0, max_attempts)]) + + self.assertRaises(nsxlib_exc.ServiceUnavailable, + api.get, 'api/v1/transport-zones') + self.assertEqual(cluster.ClusterHealth.GREEN, api.health) + def test_cluster_proxy_connection_establish_error(self): def connect_timeout(): diff --git a/vmware_nsxlib/v3/client.py b/vmware_nsxlib/v3/client.py index 5dd78315..a832cb40 100644 --- a/vmware_nsxlib/v3/client.py +++ b/vmware_nsxlib/v3/client.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. # +import json import re import time @@ -29,6 +30,63 @@ LOG = log.getLogger(__name__) NULL_CURSOR_PREFIX = '0000' +def get_http_error_details(response): + try: + msg = response.json() if response.content else '' + except json.decoder.JSONDecodeError as e: + LOG.debug("decode response %s error %s", response.content, e) + # error_code can't be None else it can't be casted to an exception + return {'status_code': response.status_code, + 'error_code': response.status_code, + 'related_error_codes': [], + 'related_status_codes': [], + 'details': response.content} + + error_code = None + related_error_codes = [] + related_status_codes = [] + + if isinstance(msg, dict) and 'error_message' in msg: + error_code = msg.get('error_code') + related_errors = [error['error_message'] for error in + msg.get('related_errors', [])] + related_error_codes = [str(error['error_code']) for error in + msg.get('related_errors', []) if + error.get('error_code')] + related_status_codes = [getattr(requests.codes, error['httpStatus']) + for error in msg.get('related_errors', []) if + error.get('httpStatus')] + + details = msg.get('details') + msg = msg['error_message'] + if details: + msg += " details: %s" % details + if related_errors: + msg += " relatedErrors: %s" % ' '.join(related_errors) + + return {'status_code': response.status_code, + 'error_code': error_code, + 'related_error_codes': related_error_codes, + 'related_status_codes': related_status_codes, + 'details': msg} + + +def init_http_exception_from_response(response): + if response is None or response: + # The response object has a __bool__ method that return True for + # status code under 400. In that case there is no need for exception + return None + error_details = get_http_error_details(response) + if not error_details['error_code']: + return None + + error = http_error_to_exception(error_details['status_code'], + error_details['error_code'], + error_details['related_error_codes']) + + return error(manager='', **error_details) + + def http_error_to_exception(status_code, error_code, related_error_codes=None): errors = { requests.codes.NOT_FOUND: diff --git a/vmware_nsxlib/v3/cluster.py b/vmware_nsxlib/v3/cluster.py index 52fb5e68..b332014f 100644 --- a/vmware_nsxlib/v3/cluster.py +++ b/vmware_nsxlib/v3/cluster.py @@ -656,7 +656,10 @@ class ClusteredAPI(object): # http request/response over the wire response = do_request(url, *args, **kwargs) endpoint.set_state(EndpointState.UP) - + exc = nsx_client.init_http_exception_from_response( + response) + if exc: + raise exc return response except Exception as e: LOG.warning("Request failed due to: %s", e)