From 7cca4576f09616d6dbf7cec0d5127b04daafd248 Mon Sep 17 00:00:00 2001 From: Pablo Andres Fuente Date: Fri, 13 Dec 2013 16:51:47 -0300 Subject: [PATCH] wsgi.Resource exception handling to not log errors In order to not log some exceptions as errors, the wsgi.Resource.__call__() was modified. Instead of check if an exception is a HTTPBadRequest or a HTTPNotFound, now all the exceptions that are not childs of HTTPError are raised directly, without logging, translation or disguise. If the exception is a child of HeatException, then the exception is tranlated but not logged as an error. When this exception reaches the Fault Wrapper Middleware it is logged as debug. Some unused, or only use in test, exceptions were removed. Change-Id: I105eeb74a55dcabc79cd3926dd88da0e19d1e1f1 Closes-Bug: #1231133 --- heat/common/exception.py | 13 ---------- heat/common/wsgi.py | 9 ++++--- heat/tests/test_api_openstack_v1.py | 6 ++--- heat/tests/test_fault_middleware.py | 8 +++--- heat/tests/test_wsgi.py | 39 +++++++++++++++++++++++++++++ 5 files changed, 51 insertions(+), 24 deletions(-) diff --git a/heat/common/exception.py b/heat/common/exception.py index 5eb0551b24..2a5222b961 100644 --- a/heat/common/exception.py +++ b/heat/common/exception.py @@ -166,19 +166,10 @@ class AuthorizationRedirect(HeatException): msg_fmt = _("Redirecting to %(uri)s for authorization.") -class ClientConfigurationError(HeatException): - msg_fmt = _("There was an error configuring the client.") - - class RequestUriTooLong(HeatException): msg_fmt = _("The URI was too long.") -class ServerError(HeatException): - msg_fmt = _("The request returned 500 Internal Server Error" - "\n\nThe response body:\n%(body)s") - - class MaxRedirectsExceeded(HeatException): msg_fmt = _("Maximum redirects (%(redirects)s) was exceeded.") @@ -187,10 +178,6 @@ class InvalidRedirect(HeatException): msg_fmt = _("Received invalid HTTP redirect.") -class NoServiceEndpoint(HeatException): - msg_fmt = _("Response from Keystone does not contain a Heat endpoint.") - - class RegionAmbiguity(HeatException): msg_fmt = _("Multiple 'image' service matches for region %(region)s. This " "generally means that a region is required and you have not " diff --git a/heat/common/wsgi.py b/heat/common/wsgi.py index 6279bdab63..3c56059ddc 100644 --- a/heat/common/wsgi.py +++ b/heat/common/wsgi.py @@ -670,17 +670,18 @@ class Resource(object): # won't make it into the pipeline app that serializes errors raise exception.HTTPExceptionDisguise(http_exc) except webob.exc.HTTPException as err: - if isinstance(err, (webob.exc.HTTPOk, webob.exc.HTTPRedirection)): + if not isinstance(err, webob.exc.HTTPError): # Some HTTPException are actually not errors, they are # responses ready to be sent back to the users, so we don't # error log, disguise or translate those raise - logging.error(_("Returning %(code)s to user: %(explanation)s"), - {'code': err.code, 'explanation': err.explanation}) + if isinstance(err, webob.exc.HTTPServerError): + logging.error( + _("Returning %(code)s to user: %(explanation)s"), + {'code': err.code, 'explanation': err.explanation}) http_exc = translate_exception(err, request.best_match_language()) raise exception.HTTPExceptionDisguise(http_exc) except exception.HeatException as err: - log_exception(err, sys.exc_info()) raise translate_exception(err, request.best_match_language()) except Exception as err: log_exception(err, sys.exc_info()) diff --git a/heat/tests/test_api_openstack_v1.py b/heat/tests/test_api_openstack_v1.py index 8d5ce2e9a8..4fdb9a1607 100644 --- a/heat/tests/test_api_openstack_v1.py +++ b/heat/tests/test_api_openstack_v1.py @@ -1284,7 +1284,7 @@ class StackControllerTest(ControllerTest, HeatTestCase): def test_list_resource_types_error(self): req = self._get('/resource_types') - error = heat_exc.ServerError(body='') + error = heat_exc.ResourceTypeNotFound(type_name='') self.m.StubOutWithMock(rpc, 'call') rpc.call(req.context, self.topic, {'namespace': None, @@ -1297,8 +1297,8 @@ class StackControllerTest(ControllerTest, HeatTestCase): resp = request_with_middleware(fault.FaultWrapper, self.controller.list_resource_types, req, tenant_id=self.tenant) - self.assertEqual(resp.json['code'], 500) - self.assertEqual(resp.json['error']['type'], 'ServerError') + self.assertEqual(resp.json['code'], 404) + self.assertEqual(resp.json['error']['type'], 'ResourceTypeNotFound') self.m.VerifyAll() def test_resource_schema(self): diff --git a/heat/tests/test_fault_middleware.py b/heat/tests/test_fault_middleware.py index 6a0a0a77b5..fae3841749 100644 --- a/heat/tests/test_fault_middleware.py +++ b/heat/tests/test_fault_middleware.py @@ -39,12 +39,12 @@ class FaultMiddlewareTest(HeatTestCase): def test_openstack_exception_without_kwargs(self): wrapper = fault.FaultWrapper(None) - msg = wrapper._error(heat_exc.NoServiceEndpoint()) + msg = wrapper._error(heat_exc.StackResourceLimitExceeded()) expected = {'code': 500, - 'error': {'message': 'Response from Keystone does ' - 'not contain a Heat endpoint.', + 'error': {'message': 'Maximum resources ' + 'per stack exceeded.', 'traceback': None, - 'type': 'NoServiceEndpoint'}, + 'type': 'StackResourceLimitExceeded'}, 'explanation': 'The server has either erred or is ' 'incapable of performing the requested ' 'operation.', diff --git a/heat/tests/test_wsgi.py b/heat/tests/test_wsgi.py index 622a674aa6..4e4b91702a 100644 --- a/heat/tests/test_wsgi.py +++ b/heat/tests/test_wsgi.py @@ -20,12 +20,15 @@ import datetime import json from oslo.config import cfg import stubout +import testscenarios import webob from heat.common import exception from heat.common import wsgi from heat.tests.common import HeatTestCase +load_tests = testscenarios.load_tests_apply_scenarios + class RequestTest(HeatTestCase): @@ -226,6 +229,42 @@ class ResourceTest(HeatTestCase): self.m.VerifyAll() +class ResourceExceptionHandlingTest(HeatTestCase): + scenarios = [ + ('client_exceptions', dict( + exception=exception.StackResourceLimitExceeded, + exception_catch=exception.StackResourceLimitExceeded)), + ('webob_bad_request', dict( + exception=webob.exc.HTTPBadRequest, + exception_catch=exception.HTTPExceptionDisguise)), + ('webob_not_found', dict( + exception=webob.exc.HTTPNotFound, + exception_catch=exception.HTTPExceptionDisguise)), + ] + + def setUp(self): + super(ResourceExceptionHandlingTest, self).setUp() + + def test_resource_client_exceptions_dont_log_error(self): + class Controller(object): + def __init__(self, excpetion_to_raise): + self.excpetion_to_raise = excpetion_to_raise + + def raise_exception(self, req, body): + raise self.excpetion_to_raise() + + actions = {'action': 'raise_exception', 'body': 'data'} + env = {'wsgiorg.routing_args': [None, actions]} + request = wsgi.Request.blank('/tests/123', environ=env) + request.body = '{"foo" : "value"}' + resource = wsgi.Resource(Controller(self.exception), + wsgi.JSONRequestDeserializer(), + None) + e = self.assertRaises(self.exception_catch, resource, request) + e = e.exc if hasattr(e, 'exc') else e + self.assertNotIn(str(e), self.logger.output) + + class JSONResponseSerializerTest(HeatTestCase): def test_to_json(self):