presubmit_support for Gerrit: don't ignore 404.

Before, presubmit_support would fail with not very useful stacktrace if
Gerrit returns 404, which is usually due to missing/invalid credentials.

This CL fixes that and improves the exception message, and also improves
logic in git_cl.

R=agable@chromium.org
BUG=

Change-Id: Iae8f0c24422c46af70929c7d5d71993164887511
Reviewed-on: https://chromium-review.googlesource.com/409650
Commit-Queue: Aaron Gable <agable@chromium.org>
Reviewed-by: Aaron Gable <agable@chromium.org>

gerrit_util.py

@@ -475,12 +475,13 @@ def GetChange(host, change):
   return ReadHttpJsonResponse(CreateHttpConn(host, path))
 
 
-def GetChangeDetail(host, change, o_params=None):
+def GetChangeDetail(host, change, o_params=None, ignore_404=True):
   """Query a gerrit server for extended information about a single change."""
+  # TODO(tandrii): cahnge ignore_404 to False by default.
   path = 'changes/%s/detail' % change
   if o_params:
     path += '?%s' % '&'.join(['o=%s' % p for p in o_params])
-  return ReadHttpJsonResponse(CreateHttpConn(host, path))
+  return ReadHttpJsonResponse(CreateHttpConn(host, path), ignore_404=ignore_404)
 
 
 def GetChangeCommit(host, change, revision='current'):

git_cl.py

@@ -2495,10 +2495,13 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
     options = options or []
     issue = issue or self.GetIssue()
     assert issue, 'issue is required to query Gerrit'
-    data = gerrit_util.GetChangeDetail(self._GetGerritHost(), str(issue),
-                                       options)
-    if not data:
-      raise GerritIssueNotExists(issue, self.GetCodereviewServer())
+    try:
+      data = gerrit_util.GetChangeDetail(self._GetGerritHost(), str(issue),
+                                         options, ignore_404=False)
+    except gerrit_util.GerritError as e:
+      if e.http_status == 404:
+        raise GerritIssueNotExists(issue, self.GetCodereviewServer())
+      raise
     return data
 
   def _GetChangeCommit(self, issue=None):

presubmit_support.py

@@ -197,9 +197,16 @@ class GerritAccessor(object):
 
   def _FetchChangeDetail(self, issue):
     # Separate function to be easily mocked in tests.
-    return gerrit_util.GetChangeDetail(
-        self.host, str(issue),
-        ['ALL_REVISIONS', 'DETAILED_LABELS'])
+    try:
+      return gerrit_util.GetChangeDetail(
+          self.host, str(issue),
+          ['ALL_REVISIONS', 'DETAILED_LABELS'],
+          ignore_404=False)
+    except gerrit_util.GerritError as e:
+      if e.http_status == 404:
+        raise Exception('Either Gerrit issue %s doesn\'t exist, or '
+                        'no credentials to fetch issue details' % issue)
+      raise
 
   def GetChangeInfo(self, issue):
     """Returns labels and all revisions (patchsets) for this issue.

tests/git_cl_test.py

@@ -1458,8 +1458,12 @@ class TestGitCl(TestCase):
                    'https://chromium-review.googlesource.com/#/c/123456/1'])
 
   def test_gerrit_patch_not_exists(self):
+    def notExists(_issue, *_, **kwargs):
+      self.assertFalse(kwargs['ignore_404'])
+      raise git_cl.gerrit_util.GerritError(404, '')
+    self.mock(git_cl.gerrit_util, 'GetChangeDetail', notExists)
+
     url = 'https://chromium-review.googlesource.com'
-    self.mock(git_cl.gerrit_util, 'GetChangeDetail', lambda _, __, ___: None)
     self.calls = [
       ((['git', 'symbolic-ref', 'HEAD'],), 'master'),
       ((['git', 'symbolic-ref', 'HEAD'],), 'master'),