diff --git a/apis/bugzilla_api.py b/apis/bugzilla_api.py index 46790ad..c7bde18 100644 --- a/apis/bugzilla_api.py +++ b/apis/bugzilla_api.py @@ -30,6 +30,55 @@ def task_log_url(): return root.rstrip('/') + '/tasks/' + os.environ['TASK_ID'] +class BugzillaGatewayError(Exception): + """Bugzilla's front end failed the request (e.g. '504 stream timeout'). The request + may or may not have been processed.""" + pass + + +# Bugzilla's front end answers with these when a request times out or doesn't reach +# Bugzilla. The ones we've seen weren't processed, but that can't be relied on, so +# before retrying a write we check whether the failed attempt went through. +GATEWAY_STATUSES = (502, 503, 504) + + +def _raise_if_gateway_error(r, where): + if getattr(r, "status_code", None) not in GATEWAY_STATUSES: + return + message = "%s: Bugzilla's front end failed the request (status=%s): %s" % (where, r.status_code, r.text[:2000]) + # We can only tell whether a write went through from the task id it was marked + # with, so outside of Taskcluster we don't retry it + if "TASK_ID" not in os.environ: + raise Exception(message) + raise BugzillaGatewayError(message) + + +def _bug_filed_by_this_task(url, apikey, summary): + r = requests.get(url + "bug", params={ + 'api_key': apikey, + 'f1': 'status_whiteboard', 'o1': 'substring', 'v1': 'task_id: ' + os.environ['TASK_ID'], + 'f2': 'short_desc', 'o2': 'equals', 'v2': summary, + 'include_fields': 'id'}) + # A gateway error here is retried too, and the next attempt checks again + _raise_if_gateway_error(r, "fileBug check") + bugs = _load_json_or_raise(r, "fileBug check")['bugs'] + return bugs[0]['id'] if bugs else None + + +def _comment_posted_by_this_task(url, apikey, bugID, comment): + r = requests.get(url + "bug/" + str(bugID) + "/comment", params={'api_key': apikey}) + _raise_if_gateway_error(r, "comment check") + bugs = _load_json_or_raise(r, "comment check")['bugs'] + for c in bugs.get(str(bugID), {}).get('comments', []): + if os.environ['TASK_ID'] in c.get('tags', []) and c.get('text', '').strip() == comment.strip(): + return True + return False + + +# Writes are retried only for gateway errors, with checks that they didn't go through +retry_write = retry(times=3, sleep_s=30, exceptions=(BugzillaGatewayError,)) + + class CommentRejectedError(Exception): """Bugzilla's web filter (not Bugzilla itself) refused the comment, with an empty 406 response. It has done this for comments quoting shell trace output.""" @@ -62,9 +111,15 @@ def _load_json_or_raise(r, where: str): ) from e -def fileBug(url, apikey, ff_version, product, component, summary, description, cc_list, needinfo, see_also, depends_on, blocks, moco_confidential, mark_affected=True): +@retry_write +def fileBug(url, apikey, ff_version, product, component, summary, description, cc_list, needinfo, see_also, depends_on, blocks, moco_confidential, mark_affected=True, retry_attempt=1): assert isinstance(cc_list, list) + if retry_attempt > 1: + filed_id = _bug_filed_by_this_task(url, apikey, summary) + if filed_id: + return filed_id + data = { 'version': "unspecified", 'op_sys': "unspecified", @@ -102,6 +157,7 @@ def fileBug(url, apikey, ff_version, product, component, summary, description, c }) r = requests.post(url + "bug?api_key=" + apikey, json=data) + _raise_if_gateway_error(r, "fileBug") j = _load_json_or_raise(r, "fileBug") @@ -111,7 +167,11 @@ def fileBug(url, apikey, ff_version, product, component, summary, description, c raise Exception(j) -def commentOnBug(url, apikey, bugID, comment, needinfo=None, assignee=None): +@retry_write +def commentOnBug(url, apikey, bugID, comment, needinfo=None, assignee=None, retry_attempt=1): + if retry_attempt > 1 and _comment_posted_by_this_task(url, apikey, bugID, comment): + return + data = { 'id': bugID, 'comment': {'body': comment}, @@ -132,6 +192,7 @@ def commentOnBug(url, apikey, bugID, comment, needinfo=None, assignee=None): json=data ) + _raise_if_gateway_error(r, "commentOnBug") if r.status_code == 406: raise CommentRejectedError("Bugzilla rejected the comment on Bug %s (status=406)" % bugID) @@ -145,8 +206,12 @@ def commentOnBug(url, apikey, bugID, comment, needinfo=None, assignee=None): raise Exception(j) -def closeBug(url, apikey, bugID, resolution, comment, dup_id=None): +@retry_write +def closeBug(url, apikey, bugID, resolution, comment, dup_id=None, retry_attempt=1): assert dup_id is None or resolution == 'DUPLICATE' + # Closing the bug and its comment happen together, so finding the comment means it closed + if retry_attempt > 1 and _comment_posted_by_this_task(url, apikey, bugID, comment): + return data = { 'id': bugID, 'status': 'RESOLVED', @@ -163,6 +228,7 @@ def closeBug(url, apikey, bugID, resolution, comment, dup_id=None): json=data ) + _raise_if_gateway_error(r, "closeBug") j = _load_json_or_raise(r, "closeBug") if 'bugs' in j: @@ -200,6 +266,7 @@ def openBugsMetadata(url, bugIDs): raise Exception(j) from e +@retry_write def markFFVersionAffected(url, apikey, bugID, ff_version, affected): if not is_prod(url): return @@ -214,6 +281,8 @@ def markFFVersionAffected(url, apikey, bugID, ff_version, affected): json=data ) + # Setting a field again is harmless, so a retry needs no check + _raise_if_gateway_error(r, "markFFVersionAffected") j = _load_json_or_raise(r, "markFFVersionAffected") if 'bugs' in j: diff --git a/apis/phabricator.py b/apis/phabricator.py index 93e1e50..77cf994 100644 --- a/apis/phabricator.py +++ b/apis/phabricator.py @@ -207,5 +207,8 @@ def abandon(self, phab_revision): if result['error']: if "You can not abandon this revision because it has already been closed." in result['errorMessage']: self.logger.log("Strangely, the phabricator revision %s was already closed when we tried to abandon it. Oh well." % phab_revision, level=LogLevel.Warning) + elif "You can not abandon this revision because you are not the author." in result['errorMessage']: + # Someone commandeered the revision, so it's theirs to keep or abandon + self.logger.log("The phabricator revision %s has been commandeered by someone else, so we're leaving it alone instead of abandoning it." % phab_revision, level=LogLevel.Info) else: raise Exception("Got an error from phabricator when trying to abandon %s: %s" % (phab_revision, result)) diff --git a/tests/bugzilla.py b/tests/bugzilla.py index ea77ede..051abcf 100755 --- a/tests/bugzilla.py +++ b/tests/bugzilla.py @@ -8,6 +8,7 @@ import re import sys import unittest +import urllib.parse from threading import Thread from http import server @@ -19,6 +20,7 @@ from components.bugzilla import BugzillaProvider, CommentTemplates from components.logging import SimpleLoggerConfig +import components.utilities from apis.bugzilla_api import task_id_whiteboard, CommentRejectedError TRY_REVISION = "this-is-my-try-link" @@ -28,12 +30,37 @@ class MockBugzillaServer(server.BaseHTTPRequestHandler): closed_bug_ids = [] filed_bugs = [] received_comments = {} + # Writes that failed with a gateway error, but that Bugzilla processed anyway + processed_bugs = {} + processed_comments = {} + comment_gets = {} + + def _gateway_error(self, status): + self.send_response(status) + self.send_header("Content-type", "text/plain") + self.end_headers() + self.wfile.write(b"stream timeout") def do_POST(self): expectedPath_file = "/bug?api_key=bob" size = int(self.headers.get('content-length')) content = json.loads(self.rfile.read(size).decode("utf-8")) + # Filing fails with a 502 the first time; 'gateway-processed' was filed anyway + if 'gateway-' in content.get('summary', ''): + MockBugzillaServer.filed_bugs.append(content) + attempts = len([b for b in MockBugzillaServer.filed_bugs if b['summary'] == content['summary']]) + if attempts == 1: + if 'gateway-processed' in content['summary']: + MockBugzillaServer.processed_bugs[content['summary']] = 460 + self._gateway_error(502) + return + self.send_response(200) + self.send_header("Content-type", "application/json") + self.end_headers() + self.wfile.write(b'{"id":459}') + return + self.send_response(200) self.send_header("Content-type", "application/json") self.end_headers() @@ -83,11 +110,26 @@ def do_GET(self): expectedPath_comments_github = "/bug/7893/comment" expectedPath_comments_mozilla_central = "/bug/7894/comment" + # Checking bug 1233's comments fails with a gateway error the first time + if self.path.startswith("/bug/1233/comment"): + MockBugzillaServer.comment_gets["1233"] = MockBugzillaServer.comment_gets.get("1233", 0) + 1 + if MockBugzillaServer.comment_gets["1233"] == 1: + self._gateway_error(504) + return + self.send_response(200) self.send_header("Content-type", "application/json") self.end_headers() - if expectedPath_find in self.path: + query = urllib.parse.parse_qs(urllib.parse.urlparse(self.path).query) + if query.get('f1') == ['status_whiteboard']: + # Looking for a bug filed by this task with this summary + bug_id = MockBugzillaServer.processed_bugs.get(query['v2'][0]) + self.wfile.write(json.dumps({'bugs': [{'id': bug_id}] if bug_id else []}).encode()) + elif re.match(r"/bug/(1231|1233|7895)/comment", self.path): + bug_id = re.match(r"/bug/([0-9]+)/", self.path).group(1) + self.wfile.write(json.dumps({'bugs': {bug_id: {'comments': MockBugzillaServer.processed_comments.get(bug_id, [])}}}).encode()) + elif expectedPath_find in self.path: self.wfile.write('{"bugs":[{"id":2,"assigned_to_detail":{"id":578488,"email":"tom@mozilla.com","real_name":"Tom Ritter [:tjr]","name":"tom@mozilla.com","nick":"tjr"},"assigned_to":"tom@mozilla.com"}],"faults":[]}'.encode()) elif expectedPath_comments_autoland == self.path: self.wfile.write('{"bugs":{"7892":{"comments":[{"text":"landing: https://hg.mozilla.org/integration/autoland/rev/abc123"}]}}}'.encode()) @@ -109,6 +151,16 @@ def do_PUT(self): bug_id = re.match(r"/bug/([0-9]+)\?api_key=bob", self.path).groups(0)[0] MockBugzillaServer.received_comments.setdefault(bug_id, []).append(content) + # Gateway errors: bugs 1230 and 1233 fail once, bug 1231 fails but is processed + # anyway, bug 1232 always fails, and closing bug 7895 fails but is processed anyway + attempts = len(MockBugzillaServer.received_comments[bug_id]) + if bug_id in ("1231", "7895"): + MockBugzillaServer.processed_comments.setdefault(bug_id, []).append( + {'text': content['comment']['body'], 'tags': content['comment_tags']}) + if (bug_id in ("1230", "1233") and attempts == 1) or bug_id in ("1231", "1232", "7895"): + self._gateway_error(504) + return + # Like Bugzilla's web filter: an empty 406. Bug 1238 rejects everything # but Updatebot's short replacement comment; bug 1239 rejects everything. if bug_id == "1239" or (bug_id == "1238" and "Bugzilla rejected it" not in content['comment']['body']): @@ -274,6 +326,51 @@ def testVendorCommandsIncludeTheRevision(self): ]: self.assertIn("`%s`" % command, comment) + def _with_task_id(self, func): + os.environ['TASK_ID'] = 'abc123' + original_sleep = components.utilities.time.sleep + components.utilities.time.sleep = lambda s: None + try: + return func() + finally: + components.utilities.time.sleep = original_sleep + del os.environ['TASK_ID'] + + def testCommentRetriedAfterGatewayError(self): + self._with_task_id(lambda: self.bugzillaProvider.comment_on_bug(1230, "Retried comment")) + self.assertEqual(len(MockBugzillaServer.received_comments["1230"]), 2) + + def testCommentNotRepeatedIfProcessed(self): + # The comment failed with a 504 but Bugzilla posted it, so posting it again would duplicate it + self._with_task_id(lambda: self.bugzillaProvider.comment_on_bug(1231, "Processed comment")) + self.assertEqual(len(MockBugzillaServer.received_comments["1231"]), 1) + + def testCommentGatewayErrorGivesUp(self): + with self.assertRaisesRegex(Exception, "status=504"): + self._with_task_id(lambda: self.bugzillaProvider.comment_on_bug(1232, "Failing comment")) + self.assertEqual(len(MockBugzillaServer.received_comments["1232"]), 3) + + def testFileRetriedAfterGatewayError(self): + library = Struct(name='gw', bugzilla_product='Core', bugzilla_component='ImageLib') + self.assertEqual(self._with_task_id(lambda: self.bugzillaProvider.file_bug(library, "Update gw to gateway-retry", "", [])), 459) + self.assertEqual(len([b for b in MockBugzillaServer.filed_bugs if b['summary'] == "Update gw to gateway-retry"]), 2) + + def testFileNotRepeatedIfProcessed(self): + library = Struct(name='gw', bugzilla_product='Core', bugzilla_component='ImageLib') + self.assertEqual(self._with_task_id(lambda: self.bugzillaProvider.file_bug(library, "Update gw to gateway-processed", "", [])), 460) + self.assertEqual(len([b for b in MockBugzillaServer.filed_bugs if b['summary'] == "Update gw to gateway-processed"]), 1) + + def testGatewayErrorWhileCheckingIsRetried(self): + # The comment fails, then so does checking whether it was posted; the last + # attempt checks again, finds it wasn't, and posts it + self._with_task_id(lambda: self.bugzillaProvider.comment_on_bug(1233, "Retried comment")) + self.assertEqual(MockBugzillaServer.comment_gets["1233"], 2) + self.assertEqual(len(MockBugzillaServer.received_comments["1233"]), 2) + + def testCloseNotRepeatedIfProcessed(self): + self._with_task_id(lambda: self.bugzillaProvider.wontfix_bug(7895, "Closing")) + self.assertEqual(len(MockBugzillaServer.received_comments["7895"]), 1) + def testCommentRejectedWithoutTaskId(self): self.assertIn("log of the Updatebot run", CommentTemplates.COMMENT_REJECTED(None)) diff --git a/tests/functionality_all_platforms.py b/tests/functionality_all_platforms.py index f133957..fc97f47 100755 --- a/tests/functionality_all_platforms.py +++ b/tests/functionality_all_platforms.py @@ -5,6 +5,7 @@ # file, You can obtain one at http://mozilla.org/MPL/2.0/. import sys +import json import copy import inspect import unittest @@ -1271,6 +1272,56 @@ def abandon_callback(cmd): finally: self._cleanup(u, expected_values) + # Create -> Build Failed, but someone commandeered the Phabricator revision (Issue #397) + @logEntryExitHeaderLine + def testExistingJobBuildFailedRevisionCommandeered(self): + @treeherder_response + def treeherder(request_type, fullpath): + if request_type == TYPE_HEALTH: + return "health_build_failed.txt" + elif request_type == TYPE_BUGSUGGESTIONS: + return "bugsuggestions_build_failure.txt" + else: # TYPE_JOBS + return "jobs_build_failed.txt" + + abandon_calls = [0] + + def abandon_callback(cmd): + abandon_calls[0] += 1 + # What Phabricator answered in https://github.com/mozilla-services/updatebot/issues/397 + return json.dumps({ + "error": "ERR-CONDUIT-CORE", + "errorMessage": "ERR-CONDUIT-CORE: Validation errors:\n - You can not abandon this revision because you are not the author. You can only abandon revisions you own. You can change this behavior by adjusting the \"differential.always-allow-abandon\" setting in Config.", + "response": None}) + + library_filter = 'dav1d' + (u, expected_values, _check_jobs) = self._setup( + library_filter, + lambda b: ["55ca6286e3e4f4fba5d0448333fa99fc5a404a73|2021-02-09 15:30:04 -0500|2021-02-12 17:40:01 +0000"], + lambda: 50, # get_filed_bug_id_func, + lambda b: {}, # filed_bug_ids_func + treeherder, + command_callbacks={'abandon': abandon_callback} + ) + comments = [] + original_comment_on_bug = u.bugzillaProvider.comment_on_bug + u.bugzillaProvider.comment_on_bug = lambda bug_id, comment, *args, **kwargs: (comments.append(comment), original_comment_on_bug(bug_id, comment, *args, **kwargs)) + exceptions = [] + original_log_exception = u.logger.log_exception + u.logger.log_exception = lambda e: (exceptions.append(e), original_log_exception(e)) + + try: + u.run(library_filter=library_filter) + _check_jobs(JOBSTATUS.AWAITING_SECOND_PLATFORMS_TRY_RESULTS, JOBOUTCOME.PENDING) + + u.run(library_filter=library_filter) + _check_jobs(JOBSTATUS.DONE, JOBOUTCOME.BUILD_FAILED) + self.assertEqual(abandon_calls[0], 1, "Shouldn't retry abandoning a revision someone else owns") + self.assertEqual([c for c in comments if "error while trying to abandon" in c], [], "Shouldn't report failing to abandon a commandeered revision") + self.assertEqual(exceptions, []) + finally: + self._cleanup(u, expected_values) + # Create -> Build Failed because of an intermittent -> Retrigger the build (Issue #453) # The jobs list, push health and bug suggestions are the real Treeherder responses for # the try push of https://bugzilla.mozilla.org/show_bug.cgi?id=2089270 (harfbuzz 15.1.0):