From: Eoghan Glynn Date: Wed, 15 Aug 2012 10:36:09 +0000 (+0000) Subject: Map internal exceptions in the nova style. X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=f7938c0b96c047490b6eb935cbf882505ce4a811;p=openstack-build%2Fcinder-build.git Map internal exceptions in the nova style. Fixes LP 1014687. Adopt the nova idiom of internal exceptions mapped to their corresponding HTTP reponse code in the FaultWrapper middleware, and also inclusion of their detail message if declared safe for external exposure. Change-Id: I95a3c30466f6951e02f7a182545ff8db8fbecee8 --- diff --git a/cinder/api/openstack/__init__.py b/cinder/api/openstack/__init__.py index 2963bac41..117babd98 100644 --- a/cinder/api/openstack/__init__.py +++ b/cinder/api/openstack/__init__.py @@ -24,6 +24,7 @@ import routes import webob.dec import webob.exc +from cinder import utils from cinder.api.openstack import wsgi from cinder.openstack.common import log as logging from cinder import wsgi as base_wsgi @@ -35,20 +36,48 @@ LOG = logging.getLogger(__name__) class FaultWrapper(base_wsgi.Middleware): """Calls down the middleware stack, making exceptions into faults.""" + _status_to_type = {} + + @staticmethod + def status_to_type(status): + if not FaultWrapper._status_to_type: + for clazz in utils.walk_class_hierarchy(webob.exc.HTTPError): + FaultWrapper._status_to_type[clazz.code] = clazz + return FaultWrapper._status_to_type.get( + status, webob.exc.HTTPInternalServerError)() + + def _error(self, inner, req): + LOG.exception(_("Caught error: %s"), unicode(inner)) + + safe = getattr(inner, 'safe', False) + headers = getattr(inner, 'headers', None) + status = getattr(inner, 'code', 500) + if status is None: + status = 500 + + 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) + if headers: + outer.headers = headers + # NOTE(johannes): We leave the explanation empty here on + # purpose. It could possibly have sensitive information + # that should not be returned back to the user. See + # bugs 868360 and 874472 + # NOTE(eglynn): However, it would be over-conservative and + # 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)) + return wsgi.Fault(outer) + @webob.dec.wsgify(RequestClass=wsgi.Request) def __call__(self, req): try: return req.get_response(self.application) except Exception as ex: - LOG.exception(_("Caught error: %s"), unicode(ex)) - msg_dict = dict(url=req.url, status=500) - LOG.info(_("%(url)s returned with HTTP %(status)d") % msg_dict) - exc = webob.exc.HTTPInternalServerError() - # NOTE(johannes): We leave the explanation empty here on - # purpose. It could possibly have sensitive information - # that should not be returned back to the user. See - # bugs 868360 and 874472 - return wsgi.Fault(exc) + return self._error(ex, req) class APIMapper(routes.Mapper): diff --git a/cinder/exception.py b/cinder/exception.py index 298edc62c..242da22f9 100644 --- a/cinder/exception.py +++ b/cinder/exception.py @@ -142,6 +142,9 @@ class CinderException(Exception): """ message = _("An unknown exception occurred.") + code = 500 + headers = {} + safe = False def __init__(self, message=None, **kwargs): self.kwargs = kwargs @@ -411,6 +414,7 @@ class InvalidUUID(Invalid): class NotFound(CinderException): message = _("Resource could not be found.") code = 404 + safe = True class FlagNotSet(NotFound): @@ -870,6 +874,9 @@ class WillNotSchedule(CinderException): class QuotaError(CinderException): message = _("Quota exceeded") + ": code=%(code)s" + code = 413 + headers = {'Retry-After': 0} + safe = True class AggregateError(CinderException): diff --git a/cinder/tests/test_wsgi.py b/cinder/tests/test_wsgi.py index cc8fb6874..09fff2399 100644 --- a/cinder/tests/test_wsgi.py +++ b/cinder/tests/test_wsgi.py @@ -22,8 +22,11 @@ import os.path import tempfile import unittest +import webob.dec -import cinder.exception +from cinder.api import openstack as openstack_api +from cinder import exception +from cinder.volume import xiv from cinder import test import cinder.wsgi @@ -90,3 +93,78 @@ class TestWSGIServer(unittest.TestCase): self.assertNotEqual(0, server.port) server.stop() server.wait() + + +class ExceptionTest(test.TestCase): + + def _wsgi_app(self, inner_app): + return openstack_api.FaultWrapper(inner_app) + + def _do_test_exception_safety_reflected_in_faults(self, expose): + class ExceptionWithSafety(exception.CinderException): + safe = expose + + @webob.dec.wsgify + def fail(req): + raise ExceptionWithSafety('some explanation') + + api = self._wsgi_app(fail) + resp = webob.Request.blank('/').get_response(api) + self.assertTrue('{"computeFault' in resp.body, resp.body) + expected = ('ExceptionWithSafety: some explanation' if expose else + 'The server has either erred or is incapable ' + 'of performing the requested operation.') + self.assertTrue(expected in resp.body, resp.body) + self.assertEqual(resp.status_int, 500, resp.body) + + def test_safe_exceptions_are_described_in_faults(self): + self._do_test_exception_safety_reflected_in_faults(True) + + 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, msg): + @webob.dec.wsgify + def fail(req): + raise exception_type(msg) + + api = self._wsgi_app(fail) + resp = webob.Request.blank('/').get_response(api) + self.assertTrue(msg in resp.body, resp.body) + self.assertEqual(resp.status_int, exception_type.code, resp.body) + + if hasattr(exception_type, 'headers'): + for (key, value) in exception_type.headers.iteritems(): + 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, 'too many used') + + def test_non_cinder_notfound_exception_mapping(self): + class ExceptionWithCode(Exception): + code = 404 + + self._do_test_exception_mapping(ExceptionWithCode, + 'NotFound') + + def test_non_cinder_exception_mapping(self): + class ExceptionWithCode(Exception): + code = 417 + + self._do_test_exception_mapping(ExceptionWithCode, + 'Expectation failed') + + def test_exception_with_none_code_throws_500(self): + class ExceptionWithNoneCode(Exception): + code = None + + msg = 'Internal Server Error' + + @webob.dec.wsgify + def fail(req): + raise ExceptionWithNoneCode() + + api = self._wsgi_app(fail) + resp = webob.Request.blank('/').get_response(api) + self.assertEqual(500, resp.status_int) diff --git a/cinder/utils.py b/cinder/utils.py index 3819047c5..2675ab619 100644 --- a/cinder/utils.py +++ b/cinder/utils.py @@ -1256,6 +1256,19 @@ def strcmp_const_time(s1, s2): return result == 0 +def walk_class_hierarchy(clazz, encountered=None): + """Walk class hierarchy, yielding most derived classes first""" + if not encountered: + encountered = [] + for subclass in clazz.__subclasses__(): + if subclass not in encountered: + encountered.append(subclass) + # drill down to leaves first + for subsubclass in walk_class_hierarchy(subclass, encountered): + yield subsubclass + yield subclass + + class UndoManager(object): """Provides a mechanism to facilitate rolling back a series of actions when an exception is raised.