CQ: handle HTTP errors when accessing Rietveld

commit-queue/verification/try_job_on_rietveld.py

Index: commit-queue/verification/try_job_on_rietveld.py
===================================================================
--- commit-queue/verification/try_job_on_rietveld.py	(revision 227588)
+++ commit-queue/verification/try_job_on_rietveld.py	(working copy)
@@ -633,19 +633,15 @@
       return
     self.last_update = now
 
-    # First, update the status of the current pending CLs on Rietveld.
+    # Update the status of the current pending CLs on Rietveld.
     for pending, jobs in self.loop(queue, RietveldTryJobs, True):
-      self._update_jobs_from_rietveld(pending, jobs, True, now)
+      # Update 'now' since querying the try jobs may take a significant amount
+      # of time.
+      now = time.time()
+      if self._update_jobs_from_rietveld(pending, jobs, True, now):
+        # Send any necessary job. Noop if not needed.
+        self._send_jobs(pending, jobs, now)
 
-    # Loop again serially on the PendingCommits to determine if new try jobs
-    # should be sent.
-    # Update 'now' since querying the try jobs may take a significant amount of
-    # time.
-    now = time.time()
-    for pending, jobs in self.loop(queue, RietveldTryJobs, True):
-      # Send any necessary job. Noop if not needed.
-      self._send_jobs(pending, jobs, now)
-
   def _add_pending_job_and_send_if_needed(self, builder, steps, jobs,
                                           send_job, pending, now):
     # Find if there was a previous try.
@@ -773,12 +769,28 @@
     """Grabs data from Rietveld and pass it to
     RietveldTryJobs.update_jobs_from_rietveld().
 
-    NOTE: While it is tagged as immutable, it is because it doesn't mutate
-    TryRunnerRietveld itself but note that it does mutate 'pending'.
+    Returns True on success.
     """
     status = buildbot_json.Buildbot(self.try_server_url)
-    data = self.context.rietveld.get_patchset_properties(
-        pending.issue, pending.patchset)
+    try:
+      data = self.context.rietveld.get_patchset_properties(
+          pending.issue, pending.patchset)
+    except urllib2.HTTPError as e:
+      if e.code == 404:
+        # TODO(phajdan.jr): Maybe generate a random id to correlate the user's
+        # error message and exception in the logs.
+        # Don't put exception traceback in the user-visible message to avoid
+        # leaking sensitive CQ data (passwords etc).
+        jobs.error_message = ('Failed to get patchset properties (patchset '
+                              'not found?)')
+        logging.error(str(e))
+        return False
+      elif e.code == 500:
+        # Temporary AppEngine hiccup. Just log it and return failure.
+        logging.warning(str(e))
+        return False
+      else:
+        raise
     # Update the RietvedTryJobs object.
     keys = jobs.update_jobs_from_rietveld(
         pending.owner, pending.issue, data, status, self.context.checkout, now)
@@ -787,3 +799,5 @@
         job = jobs.try_jobs[updated_key]
         self._update_dashboard(pending, job, status)
         jobs.signal_as_failed_if_needed(job, self._build_status_url(job), now)
+
+    return True