Exceptions in the OpenStack API will be converted to Faults as they should be, rather than barfing a stack trace to the user.

5 files changed, 81 insertions, 3 deletions

diff --git a/nova/api/openstack/__init__.py b/nova/api/openstack/__init__.py
index bb86f08dd..1dd3ba770 100644
--- a/nova/api/openstack/__init__.py
+++ b/nova/api/openstack/__init__.py
@@ -23,6 +23,7 @@ WSGI middleware for OpenStack API controllers.
 import json
 import time
 
+import logging
 import routes
 import webob.dec
 import webob.exc
@@ -54,6 +55,15 @@ class API(wsgi.Middleware):
         app = AuthMiddleware(RateLimitingMiddleware(APIRouter()))
         super(API, self).__init__(app)
 
+    @webob.dec.wsgify
+    def __call__(self, req):
+        try:
+            return req.get_response(self.application)
+        except Exception as ex:
+            logging.warn("Caught error: %s" % str(ex))
+            exc = webob.exc.HTTPInternalServerError(explanation=str(ex))
+            return faults.Fault(exc)
+
 
 class AuthMiddleware(wsgi.Middleware):
     """Authorize the openstack API request or return an HTTP Forbidden."""
diff --git a/nova/api/openstack/ratelimiting/__init__.py b/nova/api/openstack/ratelimiting/__init__.py
index 9e028ecf5..918caf055 100644
--- a/nova/api/openstack/ratelimiting/__init__.py
+++ b/nova/api/openstack/ratelimiting/__init__.py
@@ -68,10 +68,10 @@ class Limiter(object):
         self._levels[key] = (now, new_level)
         return None
 
-
 # If one instance of this WSGIApps is unable to handle your load, put a
 # sharding app in front that shards by username to one of many backends.
 
+
 class WSGIApp(object):
 
     """Application that tracks rate limits in memory.  Send requests to it of
diff --git a/nova/tests/api/openstack/test_api.py b/nova/tests/api/openstack/test_api.py
new file mode 100644
index 000000000..a8c0ff9f8
--- /dev/null
+++ b/nova/tests/api/openstack/test_api.py
@@ -0,0 +1,68 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2010 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+import unittest
+import webob.exc
+import webob.dec
+
+import nova.api.openstack
+from nova.api.openstack import API
+from nova.api.openstack import faults
+from webob import Request
+
+class APITest(unittest.TestCase):
+
+    def test_exceptions_are_converted_to_faults(self):
+        @webob.dec.wsgify
+        def succeed(req):
+            return 'Succeeded'
+        @webob.dec.wsgify
+        def raise_webob_exc(req):
+            raise webob.exc.HTTPNotFound(explanation='Raised a webob.exc')
+        @webob.dec.wsgify
+        def fail(req):
+            raise Exception("Threw an exception")
+        @webob.dec.wsgify
+        def raise_api_fault(req):
+            exc = webob.exc.HTTPNotFound(explanation='Raised a webob.exc')
+            return faults.Fault(exc)
+        api = API()
+
+        api.application = succeed
+        resp = Request.blank('/').get_response(api)
+        self.assertFalse('cloudServersFault' in resp.body, resp.body)
+        self.assertEqual(resp.status_int, 200, resp.body)
+
+        api.application = raise_webob_exc
+        resp = Request.blank('/').get_response(api)
+        self.assertFalse('cloudServersFault' in resp.body, resp.body)
+        self.assertEqual(resp.status_int, 404, resp.body)
+
+        api.application = raise_api_fault
+        resp = Request.blank('/').get_response(api)
+        self.assertTrue('itemNotFound' in resp.body, resp.body)
+        self.assertEqual(resp.status_int, 404, resp.body)
+
+        api.application = fail
+        resp = Request.blank('/').get_response(api)
+        self.assertTrue('{"cloudServersFault' in resp.body, resp.body)
+        self.assertEqual(resp.status_int, 500, resp.body)
+
+        api.application = fail
+        resp = Request.blank('/.xml').get_response(api)
+        self.assertTrue('<cloudServersFault' in resp.body, resp.body)
+        self.assertEqual(resp.status_int, 500, resp.body)
diff --git a/nova/tests/api/test_wsgi.py b/nova/tests/api/test_wsgi.py
index 604d0cb66..44e2d615c 100644
--- a/nova/tests/api/test_wsgi.py
+++ b/nova/tests/api/test_wsgi.py
@@ -112,7 +112,7 @@ class SerializerTest(unittest.TestCase):
         self.match('/servers/4.json', None, expect='json')
         self.match('/servers/4', 'application/json', expect='json')
         self.match('/servers/4', 'application/xml', expect='xml')
-        self.match('/servers/4.xml',  None, expect='xml')
+        self.match('/servers/4.xml', None, expect='xml')
 
     def test_defaults_to_json(self):
         self.match('/servers/4', None, expect='json')
diff --git a/nova/tests/api_unittest.py b/nova/tests/api_unittest.py
index 80493b10a..0b1c3e353 100644
--- a/nova/tests/api_unittest.py
+++ b/nova/tests/api_unittest.py
@@ -242,7 +242,7 @@ class ApiEc2TestCase(test.BaseTestCase):
                 self.assertEquals(int(group.rules[0].from_port), 80)
                 self.assertEquals(int(group.rules[0].to_port), 81)
                 self.assertEquals(len(group.rules[0].grants), 1)
-                self.assertEquals(str(group.rules[0].grants[0]),  '0.0.0.0/0')
+                self.assertEquals(str(group.rules[0].grants[0]), '0.0.0.0/0')
 
         self.expect_http()
         self.mox.ReplayAll()