[bugzillarest] Skip bug pages that return HTTP 400 instead of aborting

A single bug can make the Bugzilla REST API return a 400 (e.g. a field
whose data is missing from network storage:
"Failed to fetch key <n> from network storage: Not Found"). Because the
backend requests include_fields=_all and pages by last_change_time, one
such bug aborts the whole batch and freezes incremental collection
indefinitely.

Catch the HTTPError in __fetch_and_parse_bugs: on a 400, log and skip
that page (advance the offset) and continue, so one bad bug no longer
wedges collection. A MAX_CONSECUTIVE_SKIPS cap (reset on any success)
guards against an infinite skip loop on a systemic failure, re-raising
so it still surfaces.

Adds tests for both behaviours (skip-and-continue, and abort-after-cap).

(cherry picked from commit e85b333cab00f1072f0bcdb80a2d9271d8f3c123)
Signed-off-by: Arron Atchison <aatchison@thunderbird.net>

Author: Arron Atchison

perceval/backends/core/bugzillarest.py

@@ -45,6 +45,7 @@
 CATEGORY_BUG = "bug"
 MAX_BUGS = 500  # Maximum number of bugs per query
 MAX_CONTENTS = 25  # Maximum number of bug contents (history, comments) per query
+MAX_CONSECUTIVE_SKIPS = 100  # Abort after this many consecutive HTTP 400 pages
 
 
 class BugzillaREST(Backend):
@@ -184,12 +185,31 @@ def _init_client(self, from_archive=False):
     def __fetch_and_parse_bugs(self, from_date):
         max_contents = min(MAX_CONTENTS, self.max_bugs)
         offset = 0
+        consecutive_skips = 0
 
         while True:
             logger.debug("Fetching and parsing bugs from: %s, offset: %s, limit: %s ",
                          str(from_date), offset, self.max_bugs)
-            raw_bugs = self.client.bugs(from_date=from_date, offset=offset,
-                                        max_bugs=self.max_bugs)
+            try:
+                raw_bugs = self.client.bugs(from_date=from_date, offset=offset,
+                                            max_bugs=self.max_bugs)
+            except requests.exceptions.HTTPError as e:
+                # A single bug can make Bugzilla return a 400 (e.g. a field
+                # whose data is missing from network storage). Skip that page
+                # and keep going so one bad bug does not wedge the collection.
+                if e.response is not None and e.response.status_code == 400:
+                    consecutive_skips += 1
+                    if consecutive_skips > MAX_CONSECUTIVE_SKIPS:
+                        logger.error("Too many consecutive HTTP 400s (%s) fetching bugs "
+                                     "from %s; aborting", consecutive_skips, self.url)
+                        raise
+                    logger.warning("Skipping bugs page at offset %s after HTTP 400: %s",
+                                   offset, e.response.text)
+                    offset += self.max_bugs
+                    continue
+                raise
+
+            consecutive_skips = 0
 
             data = json.loads(raw_bugs)
             buglist = data['bugs']

tests/test_bugzillarest.py

@@ -29,8 +29,10 @@
 import os
 import shutil
 import unittest
+import unittest.mock
 
 import httpretty
+import requests
 
 from perceval.backend import BackendCommandArgumentParser
 from perceval.errors import BackendError
@@ -128,6 +130,87 @@ def request_callback(method, uri, headers):
     return http_requests
 
 
+def setup_http_server_with_initial_error():
+    """Like setup_http_server, but the first /rest/bug page returns HTTP 400.
+
+    Mimics a single BMO bug that 400s on include_fields=_all
+    ("Failed to fetch key ... from network storage"), which otherwise
+    wedges the whole collection.
+    """
+    http_requests = []
+
+    bodies_bugs = [read_file('data/bugzilla/bugzilla_rest_bugs.json', mode='rb'),
+                   read_file('data/bugzilla/bugzilla_rest_bugs_next.json', mode='rb'),
+                   read_file('data/bugzilla/bugzilla_rest_bugs_empty.json', mode='rb')]
+    body_comments = [read_file('data/bugzilla/bugzilla_rest_bugs_comments.json', mode='rb'),
+                     read_file('data/bugzilla/bugzilla_rest_bugs_comments_empty.json', mode='rb')]
+    body_history = [read_file('data/bugzilla/bugzilla_rest_bugs_history.json', mode='rb'),
+                    read_file('data/bugzilla/bugzilla_rest_bugs_history_empty.json', mode='rb')]
+    body_attachments = [read_file('data/bugzilla/bugzilla_rest_bugs_attachments.json', mode='rb'),
+                        read_file('data/bugzilla/bugzilla_rest_bugs_attachments_empty.json', mode='rb')]
+
+    error_body = ('{"error":true,"code":68000,'
+                  '"message":"Failed to fetch key 9327669 from network storage: Not Found"}')
+
+    def request_callback(method, uri, headers):
+        if uri.startswith(BUGZILLA_VERSION_URL):
+            body = '{"version":"5.1.2"}'
+        elif uri.startswith(BUGZILLA_BUGS_COMMENTS_1273442_URL):
+            body = body_comments[0]
+        elif uri.startswith(BUGZILLA_BUGS_HISTORY_1273442_URL):
+            body = body_history[0]
+        elif uri.startswith(BUGZILLA_BUGS_ATTACHMENTS_1273442_URL):
+            body = body_attachments[0]
+        elif uri.startswith(BUGZILLA_BUG_947945_URL):
+            if uri.find('comment') > 0:
+                body = body_comments[1]
+            elif uri.find('history') > 0:
+                body = body_history[1]
+            else:
+                body = body_attachments[1]
+        else:
+            body = bodies_bugs.pop(0)
+
+        http_requests.append(httpretty.last_request())
+
+        return (200, headers, body)
+
+    httpretty.register_uri(httpretty.GET,
+                           BUGZILLA_VERSION_URL,
+                           responses=[
+                               httpretty.Response(body=request_callback)
+                               for _ in range(3)
+                           ])
+
+    # First bug page 400s (the broken bug); the rest succeed.
+    httpretty.register_uri(httpretty.GET,
+                           BUGZILLA_BUGS_URL,
+                           responses=[
+                               httpretty.Response(body=error_body, status=400),
+                               *[httpretty.Response(body=request_callback)
+                                 for _ in range(3)]
+                           ])
+
+    http_urls = [BUGZILLA_BUGS_COMMENTS_1273442_URL,
+                 BUGZILLA_BUGS_HISTORY_1273442_URL,
+                 BUGZILLA_BUGS_ATTACHMENTS_1273442_URL]
+
+    suffixes = ['comment', 'history', 'attachment']
+
+    for http_url in [BUGZILLA_BUG_947945_URL]:
+        for suffix in suffixes:
+            http_urls.append(http_url + suffix)
+
+    for req_url in http_urls:
+        httpretty.register_uri(httpretty.GET,
+                               req_url,
+                               responses=[
+                                   httpretty.Response(body=request_callback)
+                               ])
+
+    return http_requests
+
+
 class TestBugzillaRESTBackend(unittest.TestCase):
     """Bugzilla REST backend tests"""
 
@@ -257,6 +340,46 @@ def test_fetch(self):
         for i in range(len(expected)):
             self.assertDictEqual(http_requests[i].querystring, expected[i])
 
+    @httpretty.activate
+    def test_fetch_skips_bug_page_on_http_400(self):
+        """Test whether a bug page that returns HTTP 400 is skipped
+
+        A single bug can make Bugzilla return a 400 (e.g. a field whose
+        data is missing from network storage). The whole collection must
+        not abort: the failing page is skipped and the rest are fetched.
+        """
+        setup_http_server_with_initial_error()
+
+        bg = BugzillaREST(BUGZILLA_SERVER_URL, max_bugs=2)
+        bugs = [bug for bug in bg.fetch(from_date=None)]
+
+        # The 400 page was skipped; the remaining pages still yield all bugs.
+        self.assertEqual(len(bugs), 3)
+        self.assertEqual(bugs[0]['data']['id'], 1273442)
+        self.assertEqual(bugs[1]['data']['id'], 1273439)
+        self.assertEqual(bugs[2]['data']['id'], 947945)
+
+    @httpretty.activate
+    def test_fetch_aborts_after_too_many_consecutive_400s(self):
+        """Test whether persistent HTTP 400s abort instead of looping forever"""
+
+        httpretty.register_uri(httpretty.GET,
+                               BUGZILLA_VERSION_URL,
+                               body='{"version":"5.1.2"}')
+        error_body = ('{"error":true,"code":68000,'
+                      '"message":"Failed to fetch key 9327669 from network storage: Not Found"}')
+        httpretty.register_uri(httpretty.GET,
+                               BUGZILLA_BUGS_URL,
+                               body=error_body,
+                               status=400)
+
+        bg = BugzillaREST(BUGZILLA_SERVER_URL, max_bugs=1)
+
+        with unittest.mock.patch(
+                'perceval.backends.core.bugzillarest.MAX_CONSECUTIVE_SKIPS', 3):
+            with self.assertRaises(requests.exceptions.HTTPError):
+                _ = [bug for bug in bg.fetch(from_date=None)]
+
     @httpretty.activate
     def test_search_fields(self):
         """Test whether the search_fields is properly set"""