From 55fb414b6d6929eda33822ff4e543eba0c76a9ef Mon Sep 17 00:00:00 2001 From: Michael Kedar Date: Tue, 10 Oct 2023 12:42:57 +1100 Subject: [PATCH] Raise exception when git clone fails (#1709) When a git repository failed to clone, we had just been logging an error and continuing. [The code later assumes the clone was successful](https://github.com/google/osv.dev/blob/36c344dee7e46cc1fa5cf0dea2222552b9efe953/osv/impact.py#L321), which was causing another exception. Instead, just raise an exception when the clone fails to make it more obvious what's wrong. --- osv/repos.py | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/osv/repos.py b/osv/repos.py index e720c7b283a..5477facfa1e 100644 --- a/osv/repos.py +++ b/osv/repos.py @@ -95,17 +95,26 @@ def _set_git_callback_env(git_callbacks): return env +class GitCloneError(Exception): + """Git repository clone exception.""" + + def clone(git_url, checkout_dir, git_callbacks=None): """Perform a clone.""" - # Use 'git' CLI here as it's much faster than libgit2's clone. - env = _set_git_callback_env(git_callbacks) - - subprocess.run( - ['git', 'clone', _git_mirror(git_url), checkout_dir], - env=env, - capture_output=True, - check=True) - return pygit2.Repository(checkout_dir) + try: + # Use 'git' CLI here as it's much faster than libgit2's clone. + env = _set_git_callback_env(git_callbacks) + + subprocess.run( + ['git', 'clone', _git_mirror(git_url), checkout_dir], + env=env, + capture_output=True, + check=True) + return pygit2.Repository(checkout_dir) + except subprocess.CalledProcessError as e: + raise GitCloneError(f'Failed to clone repo:\n{e.stderr.decode()}') from e + except pygit2.GitError as e: + raise GitCloneError('Failed to open cloned repo') from e def clone_with_retries(git_url, checkout_dir, git_callbacks=None, branch=None): @@ -118,17 +127,12 @@ def clone_with_retries(git_url, checkout_dir, git_callbacks=None, branch=None): if branch: _checkout_branch(repo, branch) return repo - except (pygit2.GitError, subprocess.CalledProcessError) as e: - if attempt == CLONE_TRIES - 1: - err_str = str(e) - if isinstance(e, subprocess.CalledProcessError): - # add the git output to the log - err_str = f'{err_str}\n{e.stderr.decode()}' - logging.error('Clone failed after %d attempts: %s', CLONE_TRIES, - err_str) + except GitCloneError: shutil.rmtree(checkout_dir, ignore_errors=True) + if attempt == CLONE_TRIES - 1: + logging.error('Clone failed after %d attempts', CLONE_TRIES) + raise time.sleep(RETRY_SLEEP_SECONDS) - continue return None