Python runner should throw / log an error when failing to de-serialize action result

CHANGELOG.rst

@@ -6,7 +6,15 @@ in development
 
 * Update ``st2 run`` / ``st2 execution run`` command to display result of workflow actions when
   they finish. In the workflow case, result of the last task (action) of the workflow is used.
-  (improvement)
+  (improvement) #3481
+* Update Python runner so it mimics behavior from StackStorm pre 1.6 and returns action result as
+  is (serialized as string) in case we are unable to serialize action result because it contains
+  non-simple types (e.g. class instances) which can't be serialized.
+
+  In v1.6 we introduced a change when in such instances, we simply returned ``None`` as result
+  and didn't log anything which was confusing. (improvement) #3489
+
+  Reported by Anthony Shaw.
 
 2.3.0 - June 19, 2017
 ---------------------

contrib/runners/python_runner/python_runner.py

@@ -14,6 +14,7 @@
 # limitations under the License.
 
 import os
+import re
 import sys
 import json
 import uuid
@@ -178,12 +179,23 @@ def _get_output_values(self, exit_code, stdout, stderr, timed_out):
         # Parse the serialized action result object
         try:
             action_result = json.loads(action_result)
-        except:
-            pass
+        except Exception as e:
+            # Failed to de-serialize the result, probably it contains non-simple type or similar
+            LOG.warning('Failed to de-serialize result "%s": %s' % (str(action_result), str(e)))
+
+        if action_result:
+            if isinstance(action_result, dict):
+                result = action_result.get('result', None)
+                status = action_result.get('status', None)
+            else:
+                # Failed to de-serialize action result aka result is a string
+                match = re.search("'result': (.*?)$", action_result or '')
+
+                if match:
+                    action_result = match.groups()[0]
 
-        if action_result and isinstance(action_result, dict):
-            result = action_result.get('result', None)
-            status = action_result.get('status', None)
+                result = action_result
+                status = None
         else:
             result = 'None'
             status = None

contrib/runners/python_runner/tests/unit/test_pythonrunner.py

@@ -14,6 +14,7 @@
 # limitations under the License.
 
 import os
+import re
 
 import mock
 
@@ -42,6 +43,8 @@
                              'packs/dummy_pack_9/actions/list_repos_doesnt_exist.py')
 ACTION_2_PATH = os.path.join(tests_base.get_fixtures_path(),
                              'packs/dummy_pack_9/actions/invalid_syntax.py')
+NON_SIMPLE_TYPE_ACTION = os.path.join(tests_base.get_resources_path(), 'packs',
+                                      'pythonactions/actions/non_simple_type.py')
 
 # Note: runner inherits parent args which doesn't work with tests since test pass additional
 # unrecognized args
@@ -59,6 +62,25 @@ def test_runner_creation(self):
         self.assertTrue(runner is not None, 'Creation failed. No instance.')
         self.assertEqual(type(runner), python_runner.PythonRunner, 'Creation failed. No instance.')
 
+    def test_action_returns_non_serializable_result(self):
+        # Actions returns non-simple type which can't be serialized, verify result is simple str()
+        # representation of the result
+        runner = python_runner.get_runner()
+        runner.action = self._get_mock_action_obj()
+        runner.runner_parameters = {}
+        runner.entry_point = NON_SIMPLE_TYPE_ACTION
+        runner.container_service = service.RunnerContainerService()
+        runner.pre_run()
+        (status, output, _) = runner.run({})
+
+        self.assertEqual(status, LIVEACTION_STATUS_SUCCEEDED)
+        self.assertTrue(output is not None)
+
+        expected_result_re = (r"\[{'a': '1'}, {'h': 3, 'c': 2}, {'e': "
+                              "<non_simple_type.Test object at .*?>}\]")
+        match = re.match(expected_result_re, output['result'])
+        self.assertTrue(match)
+
     def test_simple_action_with_result_no_status(self):
         runner = python_runner.get_runner()
         runner.action = self._get_mock_action_obj()

st2tests/st2tests/resources/packs/pythonactions/actions/non_simple_type.py

@@ -0,0 +1,15 @@
+from st2common.runners.base_action import Action
+
+
+class Test(object):
+    foo = 'bar'
+
+
+class NonSimpleTypeAction(Action):
+    def run(self):
+        result = [
+            {'a': '1'},
+            {'c': 2, 'h': 3},
+            {'e': Test()}
+        ]
+        return result