Fix bot_update to correctly handle exceptions

If an exception occurs during a try block which prevents a step from actually executing, then the finally block's attempt to access step.active_result will fail, and the original exception will be hidden behind a new exception from the recipe engine. This CL changes this behavior so that the original exception will always fire.

Review-Url: https://codereview.chromium.org/2175103002

recipe_modules/bot_update/api.py

@@ -231,61 +231,67 @@ class BotUpdateApi(recipe_api.RecipeApi):
       name += ' - %s' % suffix
 
     # Ah hah! Now that everything is in place, lets run bot_update!
+    step_result = None
     try:
       # 87 and 88 are the 'patch failure' codes for patch download and patch
       # apply, respectively. We don't actually use the error codes, and instead
       # rely on emitted json to determine cause of failure.
-      self(name, cmd, step_test_data=step_test_data,
+      step_result = self(name, cmd, step_test_data=step_test_data,
            ok_ret=(0, 87, 88), **kwargs)
+    except self.m.step.StepFailure as f:
+      step_result = f.result
+      raise
     finally:
-      step_result = self.m.step.active_result
-      self._last_returned_properties = step_result.json.output.get(
-          'properties', {})
-
-      if update_presentation:
-        # Set properties such as got_revision.
-        for prop_name, prop_value in self.last_returned_properties.iteritems():
-          step_result.presentation.properties[prop_name] = prop_value
-      # Add helpful step description in the step UI.
-      if 'step_text' in step_result.json.output:
-        step_text = step_result.json.output['step_text']
-        step_result.presentation.step_text = step_text
-      # Add log line output.
-      if 'log_lines' in step_result.json.output:
-        for log_name, log_lines in step_result.json.output['log_lines']:
-          step_result.presentation.logs[log_name] = log_lines.splitlines()
-
-      # Set the "checkout" path for the main solution.
-      # This is used by the Chromium module to figure out where to look for
-      # the checkout.
-      # If there is a patch failure, emit another step that said things failed.
-      if step_result.json.output.get('patch_failure'):
-        return_code = step_result.json.output.get('patch_apply_return_code')
-        if return_code == 3:
-          # This is download failure, hence an infra failure.
-          # Sadly, python.failing_step doesn't support kwargs.
-          self.m.python.inline(
-              'Patch failure',
-              ('import sys;'
-               'print "Patch download failed. See bot_update step for details";'
-               'sys.exit(1)'),
-              infra_step=True,
-              step_test_data=lambda: self.m.raw_io.test_api.output(
-                'Patch download failed. See bot_update step for details',
-                retcode=1)
-              )
-        else:
-          # This is actual patch failure.
-          self.m.tryserver.set_patch_failure_tryjob_result()
-          self.m.python.failing_step(
-              'Patch failure', 'Check the bot_update step for details')
-
-      # bot_update actually just sets root to be the folder name of the
-      # first solution.
-      if step_result.json.output['did_run']:
-        co_root = step_result.json.output['root']
-        cwd = kwargs.get('cwd', self.m.path['slave_build'])
-        if 'checkout' not in self.m.path:
-          self.m.path['checkout'] = cwd.join(*co_root.split(self.m.path.sep))
+      if step_result:
+        self._last_returned_properties = step_result.json.output.get(
+            'properties', {})
+
+        if update_presentation:
+          # Set properties such as got_revision.
+          for prop_name, prop_value in (
+              self.last_returned_properties.iteritems()):
+            step_result.presentation.properties[prop_name] = prop_value
+        # Add helpful step description in the step UI.
+        if 'step_text' in step_result.json.output:
+          step_text = step_result.json.output['step_text']
+          step_result.presentation.step_text = step_text
+        # Add log line output.
+        if 'log_lines' in step_result.json.output:
+          for log_name, log_lines in step_result.json.output['log_lines']:
+            step_result.presentation.logs[log_name] = log_lines.splitlines()
+
+        # Set the "checkout" path for the main solution.
+        # This is used by the Chromium module to figure out where to look for
+        # the checkout.
+        # If there is a patch failure, emit another step that said things
+        # failed.
+        if step_result.json.output.get('patch_failure'):
+          return_code = step_result.json.output.get('patch_apply_return_code')
+          if return_code == 3:
+            # This is download failure, hence an infra failure.
+            # Sadly, python.failing_step doesn't support kwargs.
+            self.m.python.inline(
+                'Patch failure',
+                ('import sys;'
+                 'print "Patch download failed. See bot_update step for'
+                 ' details";sys.exit(1)'),
+                infra_step=True,
+                step_test_data=lambda: self.m.raw_io.test_api.output(
+                  'Patch download failed. See bot_update step for details',
+                  retcode=1)
+                )
+          else:
+            # This is actual patch failure.
+            self.m.tryserver.set_patch_failure_tryjob_result()
+            self.m.python.failing_step(
+                'Patch failure', 'Check the bot_update step for details')
+
+        # bot_update actually just sets root to be the folder name of the
+        # first solution.
+        if step_result.json.output['did_run']:
+          co_root = step_result.json.output['root']
+          cwd = kwargs.get('cwd', self.m.path['slave_build'])
+          if 'checkout' not in self.m.path:
+            self.m.path['checkout'] = cwd.join(*co_root.split(self.m.path.sep))
 
     return step_result