Handle ironic exception in ironic-lib

Move the ironic exception to ironic-lib

Also include changes:
- Use _fmt_msg instead of message.
- Move function '_ensure_exception_kwargs_serializable' from
ironic exception module to ironic-lib exception module.

Change-Id: I467d22e7da76dfb30af0fe686a2eb4f8270b2740
Story: 1626578
Task: 30287

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. "
+    _msg_fmt = _("Failed to create a file system. "
-                "File system %(fs)s is not supported.")
+                 "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")

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)
+        )