From f6df32d94349925c5216a7f9f025a9d083cd87ab Mon Sep 17 00:00:00 2001 From: Mark McLoughlin Date: Wed, 7 Aug 2013 09:00:44 +0100 Subject: [PATCH] Add API for expected endpoint exceptions Review I4e7b19dc730342091fd70a717065741d56da4555 gives a lot of the background here, but the idea is that some exceptions raised by an RPC endpoint method do not indicate any sort of failure and should not be logged by the server as an error. The classic example of this is conductor's instance_get() method raising InstanceNotFound. This is perfectly normal and should not be considered an error. The new API is a decorator which you can use with RPC endpoints methods to indicate which exceptions are expected: @messaging.expected_exceptions(InstanceNotFound) def instance_get(self, context, instance_id): ... but we also need to expose the ExpectedException type itself so that direct "local" users of the endpoint class know what type will be used to wrap expected exceptions. For example, Nova has an ExceptionHelper class which unwraps the original exception from an ExpectedException and re-raises it. I've changed from client_exceptions() and ClientException to make it more clear it's intent. I felt that the "client" naming gave the impression it was intended for use on the client side. Change-Id: Ieec4600bd6b70cf31ac7925a98a517b84acada4d --- doc/source/server.rst | 4 ++ oslo/messaging/_drivers/amqpdriver.py | 7 ++-- oslo/messaging/_drivers/base.py | 2 +- oslo/messaging/_drivers/impl_fake.py | 2 +- oslo/messaging/_executors/base.py | 6 +++ oslo/messaging/rpc/__init__.py | 2 + oslo/messaging/rpc/server.py | 43 ++++++++++++++++++++- tests/test_expected_exceptions.py | 54 +++++++++++++++++++++++++++ tests/test_rabbit.py | 6 ++- 9 files changed, 118 insertions(+), 8 deletions(-) create mode 100644 tests/test_expected_exceptions.py diff --git a/doc/source/server.rst b/doc/source/server.rst index 64b19b828..933829f3a 100644 --- a/doc/source/server.rst +++ b/doc/source/server.rst @@ -12,3 +12,7 @@ Server .. autoclass:: MessageHandlingServer :members: + +.. autofunction:: expected_exceptions + +.. autoexception:: ExpectedException diff --git a/oslo/messaging/_drivers/amqpdriver.py b/oslo/messaging/_drivers/amqpdriver.py index ab77f4f78..3486a53a6 100644 --- a/oslo/messaging/_drivers/amqpdriver.py +++ b/oslo/messaging/_drivers/amqpdriver.py @@ -37,7 +37,8 @@ class AMQPIncomingMessage(base.IncomingMessage): self.msg_id = msg_id self.reply_q = reply_q - def _send_reply(self, conn, reply=None, failure=None, ending=False): + def _send_reply(self, conn, reply=None, failure=None, + ending=False, log_failure=True): if failure: failure = rpc_common.serialize_remote_exception(failure) @@ -56,9 +57,9 @@ class AMQPIncomingMessage(base.IncomingMessage): else: conn.direct_send(self.msg_id, rpc_common.serialize_msg(msg)) - def reply(self, reply=None, failure=None): + def reply(self, reply=None, failure=None, log_failure=True): with self.listener.driver._get_connection() as conn: - self._send_reply(conn, reply, failure) + self._send_reply(conn, reply, failure, log_failure=log_failure) self._send_reply(conn, ending=True) diff --git a/oslo/messaging/_drivers/base.py b/oslo/messaging/_drivers/base.py index 1dd49f4e6..0868d6d1d 100644 --- a/oslo/messaging/_drivers/base.py +++ b/oslo/messaging/_drivers/base.py @@ -33,7 +33,7 @@ class IncomingMessage(object): self.message = message @abc.abstractmethod - def reply(self, reply=None, failure=None): + def reply(self, reply=None, failure=None, log_failure=True): "Send a reply or failure back to the client." diff --git a/oslo/messaging/_drivers/impl_fake.py b/oslo/messaging/_drivers/impl_fake.py index 3b4c99e18..7b9ab6d57 100644 --- a/oslo/messaging/_drivers/impl_fake.py +++ b/oslo/messaging/_drivers/impl_fake.py @@ -31,7 +31,7 @@ class FakeIncomingMessage(base.IncomingMessage): super(FakeIncomingMessage, self).__init__(listener, ctxt, message) self._reply_q = reply_q - def reply(self, reply=None, failure=None): + def reply(self, reply=None, failure=None, log_failure=True): # FIXME: handle failure if self._reply_q: self._reply_q.put(reply) diff --git a/oslo/messaging/_executors/base.py b/oslo/messaging/_executors/base.py index 62101a759..7a58bb4f4 100644 --- a/oslo/messaging/_executors/base.py +++ b/oslo/messaging/_executors/base.py @@ -16,6 +16,8 @@ import abc import logging import sys +from oslo import messaging + _LOG = logging.getLogger(__name__) @@ -33,6 +35,10 @@ class ExecutorBase(object): reply = self.callback(incoming.ctxt, incoming.message) if reply: incoming.reply(reply) + except messaging.ExpectedException as e: + _LOG.debug('Expected exception during message handling (%s)' % + e.exc_info[1]) + incoming.reply(failure=e.exc_info, log_failure=False) except Exception: # sys.exc_info() is deleted by LOG.exception(). exc_info = sys.exc_info() diff --git a/oslo/messaging/rpc/__init__.py b/oslo/messaging/rpc/__init__.py index c69e4b083..69d0ebd87 100644 --- a/oslo/messaging/rpc/__init__.py +++ b/oslo/messaging/rpc/__init__.py @@ -15,12 +15,14 @@ __all__ = [ 'ClientSendError', + 'ExpectedException', 'NoSuchMethod', 'RPCClient', 'RPCDispatcher', 'RPCDispatcherError', 'RPCVersionCapError', 'UnsupportedVersion', + 'expected_exceptions', 'get_rpc_server', ] diff --git a/oslo/messaging/rpc/server.py b/oslo/messaging/rpc/server.py index 943400e43..8c86ce448 100644 --- a/oslo/messaging/rpc/server.py +++ b/oslo/messaging/rpc/server.py @@ -89,7 +89,13 @@ return values from the methods. By supplying a serializer object, a server can deserialize arguments from - serialize return values to - primitive types. """ -__all__ = ['get_rpc_server'] +__all__ = [ + 'get_rpc_server', + 'ExpectedException', + 'expected_exceptions', +] + +import sys from oslo.messaging.rpc import dispatcher as rpc_dispatcher from oslo.messaging import server as msg_server @@ -117,3 +123,38 @@ def get_rpc_server(transport, target, endpoints, dispatcher = rpc_dispatcher.RPCDispatcher(endpoints, serializer) return msg_server.MessageHandlingServer(transport, target, dispatcher, executor) + + +class ExpectedException(Exception): + """Encapsulates an expected exception raised by an RPC endpoint + + Merely instantiating this exception records the current exception + information, which will be passed back to the RPC client without + exceptional logging. + """ + def __init__(self): + self.exc_info = sys.exc_info() + + +def expected_exceptions(*exceptions): + """Decorator for RPC endpoint methods that raise expected exceptions. + + Marking an endpoint method with this decorator allows the declaration + of expected exceptions that the RPC server should not consider fatal, + and not log as if they were generated in a real error scenario. + + Note that this will cause listed exceptions to be wrapped in an + ExpectedException, which is used internally by the RPC sever. The RPC + client will see the original exception type. + """ + def outer(func): + def inner(*args, **kwargs): + try: + return func(*args, **kwargs) + except Exception as e: + if type(e) in exceptions: + raise ExpectedException() + else: + raise + return inner + return outer diff --git a/tests/test_expected_exceptions.py b/tests/test_expected_exceptions.py new file mode 100644 index 000000000..1585e4b0d --- /dev/null +++ b/tests/test_expected_exceptions.py @@ -0,0 +1,54 @@ + +# Copyright 2012 OpenStack Foundation +# Copyright 2013 Red Hat, Inc. +# +# 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. + +from oslo import messaging + +from tests import utils as test_utils + + +class TestExpectedExceptions(test_utils.BaseTestCase): + + def test_exception(self): + e = None + try: + try: + raise ValueError() + except Exception: + raise messaging.ExpectedException() + except messaging.ExpectedException as e: + self.assertIsInstance(e, messaging.ExpectedException) + self.assertTrue(hasattr(e, 'exc_info')) + self.assertIsInstance(e.exc_info[1], ValueError) + + def test_decorator_expected(self): + class FooException(Exception): + pass + + @messaging.expected_exceptions(FooException) + def naughty(): + raise FooException() + + self.assertRaises(messaging.ExpectedException, naughty) + + def test_decorator_unexpected(self): + class FooException(Exception): + pass + + @messaging.expected_exceptions(FooException) + def really_naughty(): + raise ValueError() + + self.assertRaises(ValueError, really_naughty) diff --git a/tests/test_rabbit.py b/tests/test_rabbit.py index 4f845b8be..31c03f566 100644 --- a/tests/test_rabbit.py +++ b/tests/test_rabbit.py @@ -60,7 +60,8 @@ class TestSendReceive(test_utils.BaseTestCase): _failure = [ ('success', dict(failure=False)), - ('failure', dict(failure=True)), + ('failure', dict(failure=True, expected=False)), + ('expected_failure', dict(failure=True, expected=True)), ] _timeout = [ @@ -134,7 +135,8 @@ class TestSendReceive(test_utils.BaseTestCase): raise ZeroDivisionError except Exception: failure = sys.exc_info() - msgs[i].reply(failure=failure) + msgs[i].reply(failure=failure, + log_failure=not self.expected) else: msgs[i].reply({'bar': msgs[i].message['foo']}) senders[i].join()