diff --git a/ironic_lib/exception.py b/ironic_lib/exception.py index ba770cfe..2dfba690 100644 --- a/ironic_lib/exception.py +++ b/ironic_lib/exception.py @@ -22,9 +22,11 @@ SHOULD include dedicated exception logging. """ -import logging +import collections from oslo_config import cfg +from oslo_log import log as logging +from oslo_serialization import jsonutils from oslo_utils import excutils import six @@ -36,7 +38,10 @@ LOG = logging.getLogger(__name__) exc_log_opts = [ cfg.BoolOpt('fatal_exception_format_errors', default=False, - help='Make exception message format errors fatal.', + help=_('Used if there is a formatting error when generating ' + 'an exception message (a programming error). If True, ' + 'raise an exception; if False, use the unformatted ' + 'message.'), deprecated_group='DEFAULT'), ] @@ -44,31 +49,84 @@ CONF = cfg.CONF CONF.register_opts(exc_log_opts, group='ironic_lib') +def _ensure_exception_kwargs_serializable(exc_class_name, kwargs): + """Ensure that kwargs are serializable + + Ensure that all kwargs passed to exception constructor can be passed over + RPC, by trying to convert them to JSON, or, as a last resort, to string. + If it is not possible, unserializable kwargs will be removed, letting the + receiver to handle the exception string as it is configured to. + + :param exc_class_name: a IronicException class name. + :param kwargs: a dictionary of keyword arguments passed to the exception + constructor. + :returns: a dictionary of serializable keyword arguments. + """ + serializers = [(jsonutils.dumps, _('when converting to JSON')), + (six.text_type, _('when converting to string'))] + exceptions = collections.defaultdict(list) + serializable_kwargs = {} + for k, v in kwargs.items(): + for serializer, msg in serializers: + try: + serializable_kwargs[k] = serializer(v) + exceptions.pop(k, None) + break + except Exception as e: + exceptions[k].append( + '(%(serializer_type)s) %(e_type)s: %(e_contents)s' % + {'serializer_type': msg, 'e_contents': e, + 'e_type': e.__class__.__name__}) + if exceptions: + LOG.error("One or more arguments passed to the %(exc_class)s " + "constructor as kwargs can not be serialized. The " + "serialized arguments: %(serialized)s. These " + "unserialized kwargs were dropped because of the " + "exceptions encountered during their " + "serialization:\n%(errors)s", + dict(errors=';\n'.join("%s: %s" % (k, '; '.join(v)) + for k, v in exceptions.items()), + exc_class=exc_class_name, + serialized=serializable_kwargs)) + # We might be able to actually put the following keys' values into + # format string, but there is no guarantee, drop it just in case. + for k in exceptions: + del kwargs[k] + return serializable_kwargs + + class IronicException(Exception): """Base Ironic Exception To correctly use this class, inherit from it and define - a 'message' property. That message will get printf'd + a '_msg_fmt' property. That _msg_fmt will get printf'd with the keyword arguments provided to the constructor. + If you need to access the message from an exception you should use + six.text_type(exc) + """ - message = _("An unknown exception occurred.") + + _msg_fmt = _("An unknown exception occurred.") code = 500 headers = {} safe = False def __init__(self, message=None, **kwargs): - self.kwargs = kwargs + self.kwargs = _ensure_exception_kwargs_serializable( + self.__class__.__name__, kwargs) if 'code' not in self.kwargs: try: self.kwargs['code'] = self.code except AttributeError: pass + else: + self.code = int(kwargs['code']) if not message: try: - message = self.message % kwargs + message = self._msg_fmt % kwargs except Exception: with excutils.save_and_reraise_exception() as ctxt: @@ -80,11 +138,24 @@ class IronicException(Exception): if not CONF.ironic_lib.fatal_exception_format_errors: # at least get the core message out if something # happened - message = self.message + message = self._msg_fmt ctxt.reraise = False super(IronicException, self).__init__(message) + def __str__(self): + """Encode to utf-8 then wsme api can consume it as well.""" + value = self.__unicode__() + if six.PY3: + # On Python 3 unicode is the same as str + return value + else: + return value.encode('utf-8') + + def __unicode__(self): + """Return a unicode representation of the exception message.""" + return six.text_type(self.args[0]) + def format_message(self): if self.__class__.__name__.endswith('_Remote'): return self.args[0] @@ -93,21 +164,21 @@ class IronicException(Exception): class InstanceDeployFailure(IronicException): - message = _("Failed to deploy instance: %(reason)s") + _msg_fmt = _("Failed to deploy instance: %(reason)s") class FileSystemNotSupported(IronicException): - message = _("Failed to create a file system. " - "File system %(fs)s is not supported.") + _msg_fmt = _("Failed to create a file system. " + "File system %(fs)s is not supported.") class InvalidMetricConfig(IronicException): - message = _("Invalid value for metrics config option: %(reason)s") + _msg_fmt = _("Invalid value for metrics config option: %(reason)s") class ServiceLookupFailure(IronicException): - message = _("Cannot find %(service)s service through multicast") + _msg_fmt = _("Cannot find %(service)s service through multicast") class ServiceRegistrationFailure(IronicException): - message = _("Cannot register %(service)s service: %(error)s") + _msg_fmt = _("Cannot register %(service)s service: %(error)s") diff --git a/ironic_lib/tests/test_exception.py b/ironic_lib/tests/test_exception.py new file mode 100644 index 00000000..b37f81cc --- /dev/null +++ b/ironic_lib/tests/test_exception.py @@ -0,0 +1,85 @@ +# 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 re + +import mock +from oslo_config import cfg +import six + +from ironic_lib import exception +from ironic_lib.tests import base + +CONF = cfg.CONF + + +class Unserializable(object): + def __str__(self): + raise NotImplementedError('nostr') + + +class TestException(exception.IronicException): + _msg_fmt = 'Some exception: %(spam)s, %(ham)s' + + +class TestIronicException(base.IronicLibTestCase): + def test___str__encoding(self): + expected = b'\xc3\xa9\xe0\xaf\xb2\xe0\xbe\x84' + if six.PY3: + expected = expected.decode('utf-8') + message = six.unichr(233) + six.unichr(0x0bf2) + six.unichr(3972) + exc = exception.IronicException(message) + self.assertEqual(expected, exc.__str__()) + + def test___str__non_string(self): + exc = exception.IronicException(42) + self.assertEqual("42", exc.__str__()) + self.assertEqual(u"42", exc.__unicode__()) + + def test___init___json_serializable(self): + exc = TestException(spam=[1, 2, 3], ham='eggs') + self.assertIn('[1, 2, 3]', six.text_type(exc)) + self.assertEqual('[1, 2, 3]', exc.kwargs['spam']) + + def test___init___string_serializable(self): + exc = TestException( + spam=type('ni', (object,), dict(a=1, b=2))(), ham='eggs' + ) + check_str = 'ni object at' + self.assertIn(check_str, six.text_type(exc)) + self.assertIn(check_str, exc.kwargs['spam']) + + @mock.patch.object(exception.LOG, 'error', autospec=True) + def test___init___invalid_kwarg(self, log_mock): + CONF.set_override('fatal_exception_format_errors', False, + group='ironic_lib') + e = TestException(spam=Unserializable(), ham='eggs') + message = log_mock.call_args[0][0] % log_mock.call_args[0][1] + self.assertIsNotNone( + re.search('spam: .*JSON.* ValueError: Circular reference detected;' + '.*string.* NotImplementedError: nostr', message) + ) + self.assertEqual({'ham': '"eggs"', 'code': 500}, e.kwargs) + + @mock.patch.object(exception.LOG, 'error', autospec=True) + def test___init___invalid_kwarg_reraise(self, log_mock): + CONF.set_override('fatal_exception_format_errors', True, + group='ironic_lib') + self.assertRaises(KeyError, TestException, spam=Unserializable(), + ham='eggs') + message = log_mock.call_args[0][0] % log_mock.call_args[0][1] + self.assertIsNotNone( + re.search('spam: .*JSON.* ValueError: Circular reference detected;' + '.*string.* NotImplementedError: nostr', message) + )