[Findit] Adding more try job error detection

appengine/findit/waterfall/monitor_try_job_pipeline.py

Index: appengine/findit/waterfall/monitor_try_job_pipeline.py
diff --git a/appengine/findit/waterfall/monitor_try_job_pipeline.py b/appengine/findit/waterfall/monitor_try_job_pipeline.py
index bd0802ffe674dca505524c17a1aade1c996b9745..2d21013d0086e5947ddcc17da8eb7c03aa2c4e85 100644
--- a/appengine/findit/waterfall/monitor_try_job_pipeline.py
+++ b/appengine/findit/waterfall/monitor_try_job_pipeline.py
@@ -69,12 +69,38 @@ class MonitorTryJobPipeline(BasePipeline):
           try_job_error.TIMEOUT)
 
     if buildbucket_response:
-      # If there is no explicit timeout or reason specified, check the last
-      # build response for errors.
+      # Check buildbucket_response.
+      buildbucket_failure_reason = buildbucket_response.get('failure_reason')
+      if buildbucket_failure_reason == 'BUILD_FAILURE':
+        # Can occurr if an exception is thrown or the disk is full.
+        return (
+            {
+                'message': 'Compile failed unexpectedly.',

[chanli, 2016/05/06 23:17:46]

I think 'BUILD FAILURE' or 'INFRA_FAILURE' are already very good messages. Do
you really need to separate them as a special cases?

[lijeffrey, 2016/05/07 00:12:45]

This is for consistency, the other errors are more decriptive such as
"Buildbucket reported an error." (which can be a variety of errors) and "No
result report was found" which is technically a placeholder for some bigger
error(s) being responsible that we hasn't been determined yet.

+                'reason': MonitorTryJobPipeline.UNKNOWN
+            },
+            try_job_error.INFRA_FAILURE

[chanli, 2016/05/06 23:17:46]

If it's because a bad revision which was fixed within the same regression range,
I don't think it should be grouped as INFRA_FAILURE

[lijeffrey, 2016/05/07 00:12:45]

We don't have a way of knowing that it's due to a bad revision though. It can
also be disk full or some other uncaught exception.

+        )
+      elif buildbucket_failure_reason == 'INFRA_FAILURE':
+        return (
+            {
+                'message': ('Try job encountered an infra issue during '
+                            'execution.'),
+                'reason': MonitorTryJobPipeline.UNKNOWN
+            },
+            try_job_error.INFRA_FAILURE
+        )
+      elif buildbucket_failure_reason:
+        return (
+            {
+                'message': buildbucket_failure_reason,
+                'reason': MonitorTryJobPipeline.UNKNOWN
+            },
+            try_job_error.UNKNOWN
+        )
+
+      # Check result_details_json for errors.
       result_details_json = json.loads(
           buildbucket_response.get('result_details_json', '{}')) or {}
-
-      # Check result_details_json for any obvious errors.
       error = result_details_json.get('error', {})
       if error:
         return (
@@ -84,19 +110,7 @@ class MonitorTryJobPipeline(BasePipeline):
             },
             try_job_error.CI_REPORTED_ERROR)
 
-      # Check the report to see if anything went wrong.
-      report = result_details_json.get('properties', {}).get('report')
-      if report:
-        if report.get('metadata', {}).get('infra_failure'):
-          # Check for any infra issues caught by the recipe.
-          return (
-              {
-                  'message': ('Try job encountered an infra issue during '
-                              'execution.'),
-                  'reason': MonitorTryJobPipeline.UNKNOWN
-              },
-              try_job_error.INFRA_FAILURE)
-      else:
+      if not result_details_json.get('properties', {}).get('report'):
         # A report should always be included as part of 'properties'. If it is
         # missing something else is wrong.
         return (
@@ -104,7 +118,8 @@ class MonitorTryJobPipeline(BasePipeline):
                 'message': 'No result report was found.',
                 'reason': MonitorTryJobPipeline.UNKNOWN
             },
-            try_job_error.UNKNOWN)
+            try_job_error.UNKNOWN
+        )
 
     return None, None