From 312b0d05231cb7ccac0721510f05f96dc354339e Mon Sep 17 00:00:00 2001 From: Tom Ritter Date: Tue, 20 Oct 2026 14:28:19 +0000 Subject: [PATCH] File the bug even if the upstream commits can't be listed. Fixes #435 Listing the upstream commits for a new bug's description means cloning the upstream repo. If that failed, the exception escaped _process_new_job after the job was created but before the bug was filed, leaving the job in CREATED with no bug. The next run cleaned it up as an unexpected state and marked it done, so the version was never reported. This happened to libvorbis when gitlab.xiph.org returned a 502. Catch the failure and file the bug anyway, with a description that says the commits couldn't be fetched and includes the error. Also add cache_clear() to @Memoize functions so tests can make check_for_update run again. --- components/bugzilla.py | 13 ++++++ components/utilities.py | 3 ++ tasktypes/vendoring.py | 19 ++++++--- tests/functionality_all_platforms.py | 60 ++++++++++++++++++++++++++++ tests/functionality_utilities.py | 2 +- 5 files changed, 91 insertions(+), 6 deletions(-) diff --git a/components/bugzilla.py b/components/bugzilla.py index 5e1a21e..4b8d950 100644 --- a/components/bugzilla.py +++ b/components/bugzilla.py @@ -14,6 +14,19 @@ def UPDATE_SUMMARY(library, new_release_version, release_timestamp): return "Update %s to new version %s from %s UTC" % ( library.name, new_release_version, release_timestamp) + @staticmethod + def UPDATE_DETAILS_WITHOUT_COMMITS(library, commit_stats, error): + return """ +Updatebot couldn't get the list of upstream commits for this update from %s, so they aren't listed here. The error was: + +> %s + +Here are the overall diff statistics. + +---------------------------------------- +%s +""" % (library.repo_url, error.strip().replace("\n", "\n> "), commit_stats) + @staticmethod def UPDATE_DETAILS(num_commits, num_new_commits, commit_stats, commit_details): new_commit_str = "" diff --git a/components/utilities.py b/components/utilities.py index 39536a8..4a4e900 100644 --- a/components/utilities.py +++ b/components/utilities.py @@ -147,6 +147,9 @@ def Memoize(func): def helper(*args, **kwargs): return memoized(*args, **kwargs) + # Like functools.lru_cache; the cache is shared by all instances, so tests + # that need a fresh call can clear it. + helper.cache_clear = memoized.memo.clear return helper diff --git a/tasktypes/vendoring.py b/tasktypes/vendoring.py index 2c1801d..b11889d 100644 --- a/tasktypes/vendoring.py +++ b/tasktypes/vendoring.py @@ -151,14 +151,23 @@ def _process_new_job(self, library, task, new_version, timestamp, most_recent_jo self.logger.set_context(library.name, created_job.id) # File the bug ------------------------ - all_upstream_commits, unseen_upstream_commits = self.scmProvider.check_for_update(library, task, new_version, most_recent_job.version if most_recent_job else None) commit_stats = self.mercurialProvider.diff_stats() - if library.should_show_commit_details: - commit_chunks = self.scmProvider.build_bug_description(all_upstream_commits, 65534 - len(commit_stats) - 220, library.repo_url, options=task.options) - else: + try: + all_upstream_commits, unseen_upstream_commits = self.scmProvider.check_for_update(library, task, new_version, most_recent_job.version if most_recent_job else None) + if library.should_show_commit_details: + commit_chunks = self.scmProvider.build_bug_description(all_upstream_commits, 65534 - len(commit_stats) - 220, library.repo_url, options=task.options) + else: + commit_chunks = [""] + bug_description = CommentTemplates.UPDATE_DETAILS(len(all_upstream_commits), len(unseen_upstream_commits), commit_stats, commit_chunks[0]) + except Exception as e: + # Listing the upstream commits means cloning the upstream repo, which + # fails if its host is down. Still file the bug: the job already exists, + # and without a bug a later run would treat this version as handled. + self.logger.log("Could not get the upstream commits for %s; filing the bug without them: %s" % (library.name, e), level=LogLevel.Warning) commit_chunks = [""] + bug_description = CommentTemplates.UPDATE_DETAILS_WITHOUT_COMMITS(library, commit_stats, _patch_exception_message(e)) - created_job.bugzilla_id = self.bugzillaProvider.file_bug(library, CommentTemplates.UPDATE_SUMMARY(library, new_version, timestamp), CommentTemplates.UPDATE_DETAILS(len(all_upstream_commits), len(unseen_upstream_commits), commit_stats, commit_chunks[0]), task.cc, blocks=task.blocking) + created_job.bugzilla_id = self.bugzillaProvider.file_bug(library, CommentTemplates.UPDATE_SUMMARY(library, new_version, timestamp), bug_description, task.cc, blocks=task.blocking) self.dbProvider.update_job_add_bug_id(created_job, created_job.bugzilla_id) for chunk in commit_chunks[1:]: self.bugzillaProvider.comment_on_bug(created_job.bugzilla_id, chunk) diff --git a/tests/functionality_all_platforms.py b/tests/functionality_all_platforms.py index 6209f4f..81237ea 100755 --- a/tests/functionality_all_platforms.py +++ b/tests/functionality_all_platforms.py @@ -332,6 +332,66 @@ def testFailsDuringVendor(self): finally: self._cleanup(u, expected_values) + def _capture_filed_bug_descriptions(self, u): + descriptions = [] + original = u.bugzillaProvider.file_bug + + def file_bug(library, summary, description, *args, **kwargs): + descriptions.append(description) + return original(library, summary, description, *args, **kwargs) + u.bugzillaProvider.file_bug = file_bug + return descriptions + + # Create -> ./mach vendor -> can't clone upstream to list the commits -> + # the bug is still filed, without them, and the job proceeds + @logEntryExitHeaderLine + def testUpstreamCloneFailsStillFilesBug(self): + library_filter = 'dav1d' + (u, expected_values, _check_jobs) = self._setup( + library_filter, + lambda b: ["try_rev|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 + AssertFalse, # treeherder_response + command_callbacks={'upstream_clone': lambda: raise_(Exception("The requested URL returned error: 502"))} + ) + descriptions = self._capture_filed_bug_descriptions(u) + # Earlier tests may have cached a successful result for these arguments. + SCMProvider.check_for_update.cache_clear() + try: + u.run(library_filter=library_filter) + _check_jobs(JOBSTATUS.AWAITING_SECOND_PLATFORMS_TRY_RESULTS, JOBOUTCOME.PENDING) + self.assertEqual(len(descriptions), 1) + self.assertIn("couldn't get the list of upstream commits", descriptions[0]) + self.assertIn("returned error: 502", descriptions[0]) + finally: + self._cleanup(u, expected_values) + + # The libvorbis case: ./mach vendor fails *and* upstream can't be cloned -> + # the bug is filed and gets the vendoring error, rather than crashing + @logEntryExitHeaderLine + def testFailsDuringVendorAndUpstreamClone(self): + library_filter = 'dav1d' + (u, expected_values, _check_jobs) = self._setup( + library_filter, + lambda b: ["try_rev|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 + AssertFalse, # treeherder_response + command_callbacks={'vendor': lambda: raise_(Exception("504 Server Error: Gateway Time-out")), + 'upstream_clone': lambda: raise_(Exception("The requested URL returned error: 502"))} + ) + SCMProvider.check_for_update.cache_clear() + try: + u.run(library_filter=library_filter) + lib = [lib for lib in u.libraryProvider.get_libraries(u.config_dictionary['General']['gecko-path']) if library_filter in lib.name][0] + j = u.dbProvider.get_job(lib, expected_values.library_new_version_id()) + self.assertEqual(JOBSTATUS.DONE, j.status) + self.assertEqual(JOBOUTCOME.COULD_NOT_VENDOR, j.outcome) + self.assertEqual(expected_values.get_filed_bug_id_func(), j.bugzilla_id) + finally: + self._cleanup(u, expected_values) + # Create -> ./mach vendor -> Fails during committing @logEntryExitHeaderLine def testFailsDuringCommit(self): diff --git a/tests/functionality_utilities.py b/tests/functionality_utilities.py index bae4093..5fbf0f0 100644 --- a/tests/functionality_utilities.py +++ b/tests/functionality_utilities.py @@ -84,7 +84,7 @@ def default_phab_submit(): (echo_str("echo {\"transactions\": [{\"type\":\"bugzilla.bug-id\""), lambda: CONDUIT_EDIT_OUTPUT), (echo_str("echo {\"transactions\": [{\"type\":\"parents.add\""), lambda: CONDUIT_EDIT_OUTPUT), ("git log -1 --oneline", lambda: "0481f1c (HEAD -> issue-115-add-revision-to-log, origin/issue-115-add-revision-to-log) Issue #115 - Add revision of updatebot to log output"), - ("git clone https://example.invalid .", lambda: ""), + ("git clone https://example.invalid .", command_callbacks.get('upstream_clone', lambda: "")), ("git merge-base", lambda: "_current"), ("git rev-parse", lambda: "abcdef1234567890abcdef1234567890abcdef12"), ("git log --pretty=%H|%ai|%ci", lambda cmd: "\n".join(expected_values.git_pretty_output_func("_current" not in cmd))),