[engine] bug fix: ensure we catch all exceptions in subprocess

We intended to catch all subprocesses exceptions, for example the test case
EngineTest.test_multiprocess_unpickleable checks SerializationError, but some
later added code wasn't protected, including line #159 that caused
#3149 test to hang.

This review fixes this by moving everything into the same try block.

This review does not fix the root cause of #3149
but next time it happens the subprocess won't die and will report the
exception back to the engine.

Testing Done:
https://travis-ci.org/peiyuwang/pants/builds/121229553 passed.

Bugs closed: 3149, 3155

Reviewed at https://rbcommons.com/s/twitter/r/3656/

src/python/pants/engine/exp/engine.py

@@ -154,27 +154,24 @@ def _execute_step(cache_save, debug, process_state, step):
   to the cache.
   """
   node_builder, storage = process_state
-
   step_id = step.step_id
-  resolved_request = storage.resolve_request(step)
-  try:
+
+  def execute():
+    resolved_request = storage.resolve_request(step)
     result = resolved_request(node_builder)
+    if debug:
+      _try_pickle(result)
+    cache_save(step, result)
+    return storage.key_for_result(result)
+
+  try:
+    return (step_id, execute())
   except Exception as e:
     # Trap any exception raised by the execution node that bubbles up, and
     # pass this back to our main thread for handling.
     logger.warn(traceback.format_exc())
     return (step_id, e)
 
-  if debug:
-    try:
-      _try_pickle(result)
-    except SerializationError as e:
-      return (step_id, e)
-
-  # Save result to cache for this step.
-  cache_save(step, result)
-  return (step_id, storage.key_for_result(result))
-
 
 def _process_initializer(node_builder, storage):
   """Another picklable top-level function that provides multi-processes' initial states.