[Findit] Adding more try job error detection

appengine/findit/waterfall/monitor_try_job_pipeline.py

 # Copyright 2015 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 from datetime import datetime
 import json
 import time
 
 from google.appengine.ext import ndb
 
@@ skipping 51 matching lines @@
     if timed_out:
       return (
           {
               'message': 'Try job monitoring was abandoned.',
               'reason': 'Timeout after %s hours' % (
                   waterfall_config.GetTryJobSettings().get('job_timeout_hours'))
           },
           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 (
             {
                 'message': 'Buildbucket reported an error.',
                 'reason': error.get('message', MonitorTryJobPipeline.UNKNOWN)
             },
             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 (
             {
                 'message': 'No result report was found.',
                 'reason': MonitorTryJobPipeline.UNKNOWN
             },
-            try_job_error.UNKNOWN)
+            try_job_error.UNKNOWN
+        )
 
     return None, None
 
   @staticmethod
   def _UpdateTryJobMetadata(try_job_data, start_time, buildbucket_build,
                             buildbucket_error, timed_out):
     buildbucket_response = {}
 
     if buildbucket_build:
       try_job_data.request_time = (
@@ skipping 116 matching lines @@
           already_set_started = True
 
       if time.time() > deadline:  # pragma: no cover
         self._UpdateTryJobMetadata(try_job_data, start_time, build, error, True)
         # Explicitly abort the whole pipeline.
         raise pipeline.Abort(
             'Try job %s timed out after %d hours.' % (
                 try_job_id, timeout_hours))
 
       time.sleep(pipeline_wait_seconds)  # pragma: no cover