[placement] use simple FaultWrapper

Prior to this patch placement was using the nova FaultWrapper
middleware as a final guard against unexpected exceptions. That
middleware does more than required and also imports stuff that
placement does not otherwise require.

The new middleware is only for exceptions that have fallen through
previous handling and need to be turned into 500 responses, formatted
to JSON, and logged.

While doing this it became clear that the limited Exception catching
in handler.py was redundant if FaultWrapper and the Microversion
middlewares (both required for the operation of the system) are
involved, so that has been removed, with a comment.

This should not require a microversion as the structure of successful
responses and expected error responses does not change. What does
change is that a 500 response will be formatted more as the api-wg
errors guideline[1] desires.

[1] http://specs.openstack.org/openstack/api-wg/guidelines/errors.html

Change-Id: Ib2c3250b964a30239f298cdf13ea2020f205f67d
Partial-Bug: #1743120

nova/api/openstack/placement/deploy.py

@@ -14,8 +14,8 @@
 import oslo_middleware
 from oslo_middleware import cors
 
-from nova.api import openstack as common_api
 from nova.api.openstack.placement import auth
+from nova.api.openstack.placement import fault_wrap
 from nova.api.openstack.placement import handler
 from nova.api.openstack.placement import microversion
 from nova.api.openstack.placement import requestlog
@@ -56,7 +56,7 @@ def deploy(conf):
     context_middleware = auth.PlacementKeystoneContext
     req_id_middleware = oslo_middleware.RequestId
     microversion_middleware = microversion.MicroversionMiddleware
-    fault_wrap = common_api.FaultWrapper
+    fault_middleware = fault_wrap.FaultWrapper
     request_log = requestlog.RequestLog
 
     application = handler.PlacementHandler()
@@ -70,7 +70,7 @@ def deploy(conf):
     # all see the same contextual information including request id and
     # authentication information.
     for middleware in (microversion_middleware,
-                       fault_wrap,
+                       fault_middleware,
                        request_log,
                        context_middleware,
                        auth_middleware,

nova/api/openstack/placement/fault_wrap.py

@@ -0,0 +1,48 @@
+# 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.
+"""Simple middleware for safely catching unexpected exceptions."""
+
+# NOTE(cdent): This is a super simplified replacement for the nova
+# FaultWrapper, which does more than placement needs.
+
+from oslo_log import log as logging
+import six
+from webob import exc
+
+from nova.api.openstack.placement import util
+
+LOG = logging.getLogger(__name__)
+
+
+class FaultWrapper(object):
+    """Turn an uncaught exception into a status 500.
+
+    Uncaught exceptions usually shouldn't happen, if it does it
+    means there is a bug in the placement service, which should be
+    fixed.
+    """
+
+    def __init__(self, application):
+        self.application = application
+
+    def __call__(self, environ, start_response):
+        try:
+            return self.application(environ, start_response)
+        except Exception as unexpected_exception:
+            LOG.exception('Placement API unexpected error: %s',
+                          unexpected_exception)
+            formatted_exception = exc.HTTPInternalServerError(
+                six.text_type(unexpected_exception))
+            formatted_exception.json_formatter = util.json_error_formatter
+            return formatted_exception.generate_response(
+                environ, start_response)

nova/api/openstack/placement/handler.py

@@ -223,12 +223,12 @@ class PlacementHandler(object):
         except exception.NotFound as exc:
             raise webob.exc.HTTPNotFound(
                 exc, json_formatter=util.json_error_formatter)
-        # Trap the HTTPNotFound that can be raised by dispatch()
+        # Remaining uncaught exceptions will rise first to the Microversion
-        # when no route is found. The exception is passed through to
+        # middleware, where any WebOb generated exceptions will be caught and
-        # the FaultWrap middleware without causing an alarming log
+        # transformed into legit HTTP error responses (with microversion
-        # message.
+        # headers added), and then to the FaultWrapper middleware which will
-        except webob.exc.HTTPNotFound:
+        # catch anything else and transform them into 500 responses.
-            raise
+        # NOTE(cdent): There should be very few uncaught exceptions which are
-        except Exception as exc:
+        # not WebOb exceptions at this stage as the handlers are contained by
-            LOG.exception("Uncaught exception")
+        # the wsgify decorator which will transform those exceptions to
-            raise
+        # responses itself.

nova/tests/unit/api/openstack/placement/test_fault_wrap.py

@@ -0,0 +1,66 @@
+# 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.
+"""Tests for the placement fault wrap middleware."""
+
+import mock
+
+from oslo_serialization import jsonutils
+import webob
+
+from nova.api.openstack.placement import fault_wrap
+from nova import test
+
+
+ERROR_MESSAGE = 'that was not supposed to happen'
+
+
+class Fault(Exception):
+    pass
+
+
+class TestFaultWrapper(test.NoDBTestCase):
+
+    @staticmethod
+    @webob.dec.wsgify
+    def failing_application(req):
+        raise Fault(ERROR_MESSAGE)
+
+    def setUp(self):
+        super(TestFaultWrapper, self).setUp()
+        self.req = webob.Request.blank('/')
+        self.environ = self.req.environ
+        self.environ['HTTP_ACCEPT'] = 'application/json'
+        self.start_response_mock = mock.MagicMock()
+        self.fail_app = fault_wrap.FaultWrapper(self.failing_application)
+
+    def test_fault_is_wrapped(self):
+        response = self.fail_app(self.environ, self.start_response_mock)
+        # response is a single member list
+        error_struct = jsonutils.loads(response[0])
+        first_error = error_struct['errors'][0]
+
+        self.assertIn(ERROR_MESSAGE, first_error['detail'])
+        self.assertEqual(500, first_error['status'])
+        self.assertEqual('Internal Server Error', first_error['title'])
+
+    def test_fault_response_headers(self):
+        self.fail_app(self.environ, self.start_response_mock)
+        call_args = self.start_response_mock.call_args
+        self.assertEqual('500 Internal Server Error', call_args[0][0])
+
+    @mock.patch("nova.api.openstack.placement.fault_wrap.LOG")
+    def test_fault_log(self, mocked_log):
+        self.fail_app(self.environ, self.start_response_mock)
+        mocked_log.exception.assert_called_once_with(
+                'Placement API unexpected error: %s',
+                mock.ANY)