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

cinder/api/openstack/__init__.py

@@ -24,6 +24,7 @@
 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 @@
 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):

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):

cinder/tests/test_wsgi.py

@@ -22,8 +22,11 @@
 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 @@ def test_start_random_port(self):
         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)

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.