Skip to content

Commit

Permalink
Handle local & remote exceptions consistently.
Browse files Browse the repository at this point in the history
Related to LP 1021373.

When the floating_ips quota threshold is exceeded, this occurs remotely
in the networking service and hence is reported back to the API service
over RPC, via a RemoteError wrapping the original QuotaError exception.

Previously the handling of such RemoteErrors in the native API was
inconsistent with exceptions raised locally within the API service.

Now in both cases, we expose the exception explantion, status etc. in the
response, if the exception type is declared as safe.

This fix is not required on master, as the Folsom RPC failure deserialization
logic renders it unnecessary.

Change-Id: Icf7dcafd1f942f5d81d12587775d6ade4a176932
  • Loading branch information
Eoghan Glynn committed Jul 18, 2012
1 parent 6e873bc commit 4b89b4f
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 12 deletions.
38 changes: 30 additions & 8 deletions nova/api/openstack/__init__.py
Expand Up @@ -25,6 +25,7 @@
import webob.exc

from nova.api.openstack import wsgi
from nova.rpc import common as rpc_common
from nova import exception
from nova import log as logging
from nova import utils
Expand All @@ -47,8 +48,18 @@ def status_to_type(status):
return FaultWrapper._status_to_type.get(
status, webob.exc.HTTPInternalServerError)()

def _error(self, inner, req, headers=None, status=500, safe=False):
LOG.exception(_("Caught error: %s"), unicode(inner))
_name_to_type = {}

@staticmethod
def name_to_type(name):
if not FaultWrapper._name_to_type:
for clazz in utils.walk_class_hierarchy(exception.NovaException):
FaultWrapper._name_to_type[clazz.__name__] = clazz
return FaultWrapper._name_to_type.get(name)

def _error(self, req, class_name, explanation,
headers=None, status=500, safe=False):
LOG.exception(_("Caught error: %s"), explanation)
msg_dict = dict(url=req.url, status=status)
LOG.info(_("%(url)s returned with HTTP %(status)d") % msg_dict)
outer = self.status_to_type(status)
Expand All @@ -62,18 +73,29 @@ def _error(self, inner, req, headers=None, status=500, safe=False):
# inconsistent with the EC2 API to hide every exception,
# including those that are safe to expose, see bug 1021373
if safe:
outer.explanation = '%s: %s' % (inner.__class__.__name__,
unicode(inner))
outer.explanation = '%s: %s' % (class_name, explanation)
return wsgi.Fault(outer)

@webob.dec.wsgify(RequestClass=wsgi.Request)
def __call__(self, req):
try:
return req.get_response(self.application)
except exception.NovaException as ex:
return self._error(ex, req, ex.headers, ex.code, ex.safe)
except Exception as ex:
return self._error(ex, req)
except rpc_common.RemoteError as e:
remote_exception_type = self.name_to_type(e.exc_type)
if remote_exception_type:
headers = remote_exception_type.headers
code = remote_exception_type.code
safe = remote_exception_type.safe
else:
headers = {}
code = 500
safe = False
return self._error(req, e.exc_type, e.value, headers, code, safe)
except exception.NovaException as e:
return self._error(req, e.__class__.__name__, unicode(e),
e.headers, e.code, e.safe)
except Exception as e:
return self._error(req, e.__class__.__name__, unicode(e))


class APIMapper(routes.Mapper):
Expand Down
25 changes: 21 additions & 4 deletions nova/tests/api/openstack/compute/test_api.py
Expand Up @@ -27,6 +27,7 @@
from nova import test
from nova.api import openstack as openstack_api
from nova.api.openstack import wsgi
from nova.rpc import common as rpc_common
from nova.tests.api.openstack import fakes


Expand Down Expand Up @@ -145,10 +146,10 @@ def test_safe_exceptions_are_described_in_faults(self):
def test_unsafe_exceptions_are_not_described_in_faults(self):
self._do_test_exception_safety_reflected_in_faults(False)

def _do_test_exception_mapping(self, exception_type):
def _do_test_exception_mapping(self, exception_value, exception_type):
@webob.dec.wsgify
def fail(req):
raise exception_type('too many used')
raise exception_value

api = self._wsgi_app(fail)
resp = webob.Request.blank('/').get_response(api)
Expand All @@ -158,8 +159,24 @@ def fail(req):
self.assertTrue(key in resp.headers)
self.assertEquals(resp.headers[key], value)

def test_quota_error_mapping(self):
self._do_test_exception_mapping(exception.QuotaError)
def test_local_quota_error_mapping(self):
exception_value = exception.QuotaError('too many used')
self._do_test_exception_mapping(exception_value, exception.QuotaError)

def test_remote_quota_error_mapping(self):
exception_value = rpc_common.RemoteError('QuotaError',
'too many used')
self._do_test_exception_mapping(exception_value, exception.QuotaError)

def test_remote_unknown_error_mapping(self):
@webob.dec.wsgify
def fail(req):
raise rpc_common.RemoteError('UnknownError', 'whatevs')

api = self._wsgi_app(fail)
resp = webob.Request.blank('/').get_response(api)
self.assertFalse('whatevs' in resp.body, resp.body)
self.assertEqual(resp.status_int, 500, resp.body)

def test_request_id_in_response(self):
req = webob.Request.blank('/')
Expand Down

0 comments on commit 4b89b4f

Please sign in to comment.