From f7a0c0b04415cb390edbd41eeeba0f90d9e4d36b Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Fri, 13 May 2016 17:24:35 -0700 Subject: [PATCH] Convert multiple exception types in the API The callback framework will collect all exceptions that occur during the notification loop and raise a single exception containing all of them. The issue with this is that the code that notifies has to manually unpack the exceptions and choose what to reraise for the API layer even if it has no other reason to catch the exception (i.e. any encountered exceptions should be fatal). If it doesn't, the server will just return a generic HTTP 500 error to the user even if the internal exception is a normal error that would convert to a 404, 409, etc. This patch makes the API exception conversion layer aware of exceptions containing other exceptions so code no longer has to catch callback failures if it has no specific error-handling logic. Multiple exceptions that translate to the same HTTP error code will be converted into one exception of the same error code with the details of each exception line-separated in the exception message. Multiple exceptions that translate to different HTTP error codes will be concatenated together line-separated with their HTTP error prefixes into an HTTP Conflict exception. If there is only a single exception in the multi exception type, the inner exception is used directly. Partially-Implements: bp/multi-l3-backends Change-Id: I528de088079b68cf284ef361fee9bd195125e0d8 --- neutron/api/api_common.py | 34 +++++++++++++++++++++++ neutron/callbacks/exceptions.py | 15 +++++++++- neutron/common/exceptions.py | 13 +++++++++ neutron/tests/unit/api/test_api_common.py | 29 +++++++++++++++++++ 4 files changed, 90 insertions(+), 1 deletion(-) diff --git a/neutron/api/api_common.py b/neutron/api/api_common.py index 0b8b07f2208..2d9e7439aa0 100644 --- a/neutron/api/api_common.py +++ b/neutron/api/api_common.py @@ -22,11 +22,13 @@ from oslo_config import cfg import oslo_i18n from oslo_log import log as logging from oslo_policy import policy as oslo_policy +from oslo_serialization import jsonutils from six.moves.urllib import parse from webob import exc from neutron._i18n import _, _LW from neutron.common import constants +from neutron.common import exceptions as n_exc from neutron import wsgi @@ -356,6 +358,38 @@ class NeutronController(object): def convert_exception_to_http_exc(e, faults, language): serializer = wsgi.JSONDictSerializer() + if isinstance(e, n_exc.MultipleExceptions): + converted_exceptions = [ + convert_exception_to_http_exc(inner, faults, language) + for inner in e.inner_exceptions] + # if no internal exceptions, will be handled as single exception + if converted_exceptions: + codes = {c.code for c in converted_exceptions} + if len(codes) == 1: + # all error codes are the same so we can maintain the code + # and just concatenate the bodies + joined_msg = "\n".join( + (jsonutils.loads(c.body)['NeutronError']['message'] + for c in converted_exceptions)) + new_body = jsonutils.loads(converted_exceptions[0].body) + new_body['NeutronError']['message'] = joined_msg + converted_exceptions[0].body = serializer.serialize(new_body) + return converted_exceptions[0] + else: + # multiple error types so we turn it into a Conflict with the + # inner codes and bodies packed in + new_exception = exceptions.Conflict() + inner_error_strings = [] + for c in converted_exceptions: + c_body = jsonutils.loads(c.body) + err = ('HTTP %s %s: %s' % ( + c.code, c_body['NeutronError']['type'], + c_body['NeutronError']['message'])) + inner_error_strings.append(err) + new_exception.msg = "\n".join(inner_error_strings) + return convert_exception_to_http_exc( + new_exception, faults, language) + e = translate(e, language) body = serializer.serialize( {'NeutronError': get_exception_data(e)}) diff --git a/neutron/callbacks/exceptions.py b/neutron/callbacks/exceptions.py index 03e994e4e5d..f0049c5c811 100644 --- a/neutron/callbacks/exceptions.py +++ b/neutron/callbacks/exceptions.py @@ -13,13 +13,14 @@ from neutron_lib import exceptions from neutron._i18n import _ +from neutron.common import exceptions as n_exc class Invalid(exceptions.NeutronException): message = _("The value '%(value)s' for %(element)s is not valid.") -class CallbackFailure(Exception): +class CallbackFailure(n_exc.MultipleExceptions): def __init__(self, errors): self.errors = errors @@ -30,6 +31,18 @@ class CallbackFailure(Exception): else: return str(self.errors) + @property + def inner_exceptions(self): + if isinstance(self.errors, list): + return [self._unpack_if_notification_error(e) for e in self.errors] + return [self._unpack_if_notification_error(self.errors)] + + @staticmethod + def _unpack_if_notification_error(exc): + if isinstance(exc, NotificationError): + return exc.error + return exc + class NotificationError(object): diff --git a/neutron/common/exceptions.py b/neutron/common/exceptions.py index 6405719ecbe..a2e2907db7b 100644 --- a/neutron/common/exceptions.py +++ b/neutron/common/exceptions.py @@ -21,6 +21,19 @@ from neutron._i18n import _ from neutron.common import _deprecate +class MultipleExceptions(Exception): + """Container for multiple exceptions encountered. + + The API layer of Neutron will automatically unpack, translate, + filter, and combine the inner exceptions in any exception derived + from this class. + """ + + def __init__(self, exceptions, *args, **kwargs): + super(MultipleExceptions, self).__init__(*args, **kwargs) + self.inner_exceptions = exceptions + + class SubnetPoolNotFound(e.NotFound): message = _("Subnet pool %(subnetpool_id)s could not be found.") diff --git a/neutron/tests/unit/api/test_api_common.py b/neutron/tests/unit/api/test_api_common.py index 5ccf4891c3c..6e63a62fb4e 100644 --- a/neutron/tests/unit/api/test_api_common.py +++ b/neutron/tests/unit/api/test_api_common.py @@ -13,10 +13,14 @@ # License for the specific language governing permissions and limitations # under the License. +from neutron_lib import exceptions +from oslo_serialization import jsonutils from testtools import matchers from webob import exc from neutron.api import api_common as common +from neutron.api.v2 import base as base_v2 +from neutron.common import exceptions as n_exc from neutron.tests import base @@ -92,3 +96,28 @@ class APICommonTestCase(base.BaseTestCase): self.controller._prepare_request_body, body, params) + + def test_convert_exception_to_http_exc_multiple_different_codes(self): + e = n_exc.MultipleExceptions([exceptions.NetworkInUse(net_id='nid'), + exceptions.PortNotFound(port_id='pid')]) + conv = common.convert_exception_to_http_exc(e, base_v2.FAULT_MAP, None) + self.assertIsInstance(conv, exc.HTTPConflict) + self.assertEqual( + ("HTTP 409 NetworkInUse: Unable to complete operation on network " + "nid. There are one or more ports still in use on the network.\n" + "HTTP 404 PortNotFound: Port pid could not be found."), + jsonutils.loads(conv.body)['NeutronError']['message']) + + def test_convert_exception_to_http_exc_multiple_same_codes(self): + e = n_exc.MultipleExceptions([exceptions.NetworkNotFound(net_id='nid'), + exceptions.PortNotFound(port_id='pid')]) + conv = common.convert_exception_to_http_exc(e, base_v2.FAULT_MAP, None) + self.assertIsInstance(conv, exc.HTTPNotFound) + self.assertEqual( + "Network nid could not be found.\nPort pid could not be found.", + jsonutils.loads(conv.body)['NeutronError']['message']) + + def test_convert_exception_to_http_exc_multiple_empty_inner(self): + e = n_exc.MultipleExceptions([]) + conv = common.convert_exception_to_http_exc(e, base_v2.FAULT_MAP, None) + self.assertIsInstance(conv, exc.HTTPInternalServerError)