From af1a55bfd2e47b0e3cd8349f0a9b1277474fee18 Mon Sep 17 00:00:00 2001 From: Akihiro Motoki Date: Fri, 4 Dec 2015 01:53:58 +0900 Subject: [PATCH] Ensure to use exception per status code for all cases Previously, only when an exception has a content with {'NeutronError': {'type': xxxx, 'message': xxxx}}, exception per status code is raised from neutronclient library. There are cases where this kind of message is not contained in exception messages, for example, some extension is loaded. Library users expect an exception is raised based on response status code and it should not depend on an exception message. This commit applies a fallback logic to map generic per-status exception to all exception types from the neutron server. Closes-Bug: #1513879 Change-Id: Ib3d0a8359aed444b12217b3404d40443d61fc2c0 --- neutronclient/tests/unit/test_cli20.py | 32 +++++++++++-------- neutronclient/v2_0/client.py | 44 +++++++++++--------------- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/neutronclient/tests/unit/test_cli20.py b/neutronclient/tests/unit/test_cli20.py index 8aa0aabe3..e3a1014eb 100644 --- a/neutronclient/tests/unit/test_cli20.py +++ b/neutronclient/tests/unit/test_cli20.py @@ -713,6 +713,8 @@ class CLITestV20ExceptionHandler(CLITestV20Base): client.exception_handler_v20, status_code, error_content) self.assertEqual(status_code, e.status_code) + self.assertEqual(expected_exception.__name__, + e.__class__.__name__) if expected_msg is None: if error_detail: @@ -780,32 +782,36 @@ class CLITestV20ExceptionHandler(CLITestV20Base): 'UnknownError', error_msg, error_detail) def test_exception_handler_v20_bad_neutron_error(self): - error_content = {'NeutronError': {'unknown_key': 'UNKNOWN'}} - self._test_exception_handler_v20( - exceptions.NeutronClientException, 500, - expected_msg={'unknown_key': 'UNKNOWN'}, - error_content=error_content) + for status_code, client_exc in exceptions.HTTP_EXCEPTION_MAP.items(): + error_content = {'NeutronError': {'unknown_key': 'UNKNOWN'}} + self._test_exception_handler_v20( + client_exc, status_code, + expected_msg="{'unknown_key': 'UNKNOWN'}", + error_content=error_content) def test_exception_handler_v20_error_dict_contains_message(self): error_content = {'message': 'This is an error message'} - self._test_exception_handler_v20( - exceptions.NeutronClientException, 500, - expected_msg='This is an error message', - error_content=error_content) + for status_code, client_exc in exceptions.HTTP_EXCEPTION_MAP.items(): + self._test_exception_handler_v20( + client_exc, status_code, + expected_msg='This is an error message', + error_content=error_content) def test_exception_handler_v20_error_dict_not_contain_message(self): + # 599 is not contained in HTTP_EXCEPTION_MAP. error_content = {'error': 'This is an error message'} - expected_msg = '%s-%s' % (500, error_content) + expected_msg = '%s-%s' % (599, error_content) self._test_exception_handler_v20( - exceptions.NeutronClientException, 500, + exceptions.NeutronClientException, 599, expected_msg=expected_msg, error_content=error_content) def test_exception_handler_v20_default_fallback(self): + # 599 is not contained in HTTP_EXCEPTION_MAP. error_content = 'This is an error message' - expected_msg = '%s-%s' % (500, error_content) + expected_msg = '%s-%s' % (599, error_content) self._test_exception_handler_v20( - exceptions.NeutronClientException, 500, + exceptions.NeutronClientException, 599, expected_msg=expected_msg, error_content=error_content) diff --git a/neutronclient/v2_0/client.py b/neutronclient/v2_0/client.py index cc323c957..08a850314 100644 --- a/neutronclient/v2_0/client.py +++ b/neutronclient/v2_0/client.py @@ -47,7 +47,7 @@ def exception_handler_v20(status_code, error_content): if isinstance(error_content, dict): error_dict = error_content.get('NeutronError') # Find real error type - bad_neutron_error_flag = False + client_exc = None if error_dict: # If Neutron key is found, it will definitely contain # a 'message' and 'type' keys? @@ -56,35 +56,29 @@ def exception_handler_v20(status_code, error_content): error_message = error_dict['message'] if error_dict['detail']: error_message += "\n" + error_dict['detail'] - except Exception: - bad_neutron_error_flag = True - if not bad_neutron_error_flag: # If corresponding exception is defined, use it. client_exc = getattr(exceptions, '%sClient' % error_type, None) - # Otherwise look up per status-code client exception - if not client_exc: - client_exc = exceptions.HTTP_EXCEPTION_MAP.get(status_code) - if client_exc: - raise client_exc(message=error_message, - status_code=status_code) - else: - raise exceptions.NeutronClientException( - status_code=status_code, message=error_message) - else: - raise exceptions.NeutronClientException(status_code=status_code, - message=error_dict) + except Exception: + error_message = "%s" % error_dict else: - message = None + error_message = None if isinstance(error_content, dict): - message = error_content.get('message') - if message: - raise exceptions.NeutronClientException(status_code=status_code, - message=message) + error_message = error_content.get('message') + if not error_message: + # If we end up here the exception was not a neutron error + error_message = "%s-%s" % (status_code, error_content) - # If we end up here the exception was not a neutron error - msg = "%s-%s" % (status_code, error_content) - raise exceptions.NeutronClientException(status_code=status_code, - message=msg) + # If an exception corresponding to the error type is not found, + # look up per status-code client exception. + if not client_exc: + client_exc = exceptions.HTTP_EXCEPTION_MAP.get(status_code) + # If there is no exception per status-code, + # Use NeutronClientException as fallback. + if not client_exc: + client_exc = exceptions.NeutronClientException + + raise client_exc(message=error_message, + status_code=status_code) class APIParamsCall(object):