Make sure not to swallow original exceptions on handling remote errors

* Also removed a custom initializer of SizeLimitExceededException with
  3 parameters in favor of creating a static method because we
  have to assume that all MistralException classes have an initializer
  with only one parameter "message" to be able to restore an exception
  on the RPC client which was raised on the RPC server.

Change-Id: Idd52bb7bb18168826644d8aaca3d96be46f8ae9e
Closes-Bug: #1750752

mistral/db/v2/sqlalchemy/models.py

@@ -65,18 +65,20 @@ def validate_long_type_length(cls, field_name, value):
         size_kb = int(sys.getsizeof(str(value)) / 1024)
 
         if size_kb > size_limit_kb:
-            LOG.error(
+            msg = (
-                "Size limit %dKB exceed for class [%s], "
+                "Field size limit exceeded"
-                "field %s of size %dKB.",
+                " [class={}, field={}, size={}KB, limit={}KB]"
-                size_limit_kb, str(cls), field_name, size_kb
+            ).format(
-            )
+                cls.__name__,
-
-            raise exc.SizeLimitExceededException(
                 field_name,
                 size_kb,
                 size_limit_kb
             )
 
+            LOG.error(msg)
+
+            raise exc.SizeLimitExceededException(msg)
+
 
 def register_length_validator(attr_name):
     """Register an event listener on the attribute.

mistral/exceptions.py

@@ -185,11 +185,6 @@ class InvalidResultException(MistralException):
 class SizeLimitExceededException(MistralException):
     http_code = 400
 
-    def __init__(self, field_name, size_kb, size_limit_kb):
-        super(SizeLimitExceededException, self).__init__(
-            "Size of '%s' is %dKB which exceeds the limit of %dKB"
-            % (field_name, size_kb, size_limit_kb))
-
 
 class CoordinationException(MistralException):
     http_code = 500

mistral/rpc/base.py

@@ -85,25 +85,31 @@ def _wrap_exception_and_reraise(exception):
 
 
 def wrap_messaging_exception(method):
-    """This decorator unwrap remote error in one of MistralException.
+    """The decorator unwraps a remote error into one of the mistral exceptions.
 
-    oslo.messaging has different behavior on raising exceptions
+    oslo.messaging has different behavior on raising exceptions depending on
-    when fake or rabbit transports are used. In case of rabbit
+    whether we use 'fake' or 'rabbit' transports. In case of 'rabbit' transport
-    transport it raises wrapped RemoteError which forwards directly
+    it raises an instance of RemoteError which forwards directly to the API.
-    to API. Wrapped RemoteError contains one of MistralException raised
+    The RemoteError instance contains one of the MistralException instances
-    remotely on Engine and for correct exception interpretation we
+    raised remotely on the RPC server side and for correct exception handling
-    need to unwrap and raise given exception and manually send it to
+    we need to unwrap and raise the original wrapped exception.
-    API layer.
     """
     def decorator(*args, **kwargs):
         try:
             return method(*args, **kwargs)
-
         except exc.MistralException:
             raise
         except (client.RemoteError, exc.KombuException, Exception) as e:
+            # Since we're going to transform the original exception
+            # we need to log it as is.
+            LOG.exception(
+                "Caught a messaging remote error."
+                " See details of the original exception."
+            )
+
             if hasattr(e, 'exc_type') and hasattr(exc, e.exc_type):
                 exc_cls = getattr(exc, e.exc_type)
+
                 raise exc_cls(e.value)
 
             _wrap_exception_and_reraise(e)

mistral/tests/unit/engine/test_execution_fields_size_limitation.py

@@ -140,7 +140,8 @@ class ExecutionFieldsSizeLimitTest(base.EngineTestCase):
         )
 
         self.assertEqual(
-            "Size of 'input' is 1KB which exceeds the limit of 0KB",
+            'Field size limit exceeded'
+            ' [class=TaskExecution, field=input, size=1KB, limit=0KB]',
             str(e)
         )
 
@@ -156,7 +157,8 @@ class ExecutionFieldsSizeLimitTest(base.EngineTestCase):
         )
 
         self.assertEqual(
-            "Size of 'input' is 1KB which exceeds the limit of 0KB",
+            'Field size limit exceeded'
+            ' [class=TaskExecution, field=input, size=1KB, limit=0KB]',
             str(e)
         )
 
@@ -170,7 +172,8 @@ class ExecutionFieldsSizeLimitTest(base.EngineTestCase):
 
         self.assertEqual(states.ERROR, wf_ex.state)
         self.assertIn(
-            "Size of 'input' is 1KB which exceeds the limit of 0KB",
+            "Field size limit exceeded"
+            " [class=TaskExecution, field=input, size=1KB, limit=0KB]",
             wf_ex.state_info
         )
 
@@ -189,7 +192,8 @@ class ExecutionFieldsSizeLimitTest(base.EngineTestCase):
         wf_ex = db_api.get_workflow_execution(wf_ex.id)
 
         self.assertIn(
-            "Size of 'output' is 1KB which exceeds the limit of 0KB",
+            'Field size limit exceeded'
+            ' [class=TaskExecution, field=output, size=1KB, limit=0KB]',
             wf_ex.state_info
         )
         self.assertEqual(states.ERROR, wf_ex.state)
@@ -211,7 +215,7 @@ class ExecutionFieldsSizeLimitTest(base.EngineTestCase):
             task_execs = wf_ex.task_executions
 
         self.assertIn(
-            'Failed to handle action completion [error=Size of',
+            'Failed to handle action completion [error=Field size',
             wf_ex.state_info
         )
         self.assertIn('wf=wf, task=task1', wf_ex.state_info)
@@ -219,7 +223,8 @@ class ExecutionFieldsSizeLimitTest(base.EngineTestCase):
         task_ex = self._assert_single_item(task_execs, name='task1')
 
         self.assertIn(
-            "Size of 'published' is 1KB which exceeds the limit of 0KB",
+            'Field size limit exceeded'
+            ' [class=TaskExecution, field=published, size=1KB, limit=0KB]',
             task_ex.state_info
         )
 
@@ -237,7 +242,8 @@ class ExecutionFieldsSizeLimitTest(base.EngineTestCase):
         )
 
         self.assertIn(
-            "Size of 'params' is 1KB which exceeds the limit of 0KB",
+            'Field size limit exceeded'
+            ' [class=TaskExecution, field=params, size=1KB, limit=0KB]',
             str(e)
         )