From dd59afe59d32fed8c95be80676cfb2562f7a78e9 Mon Sep 17 00:00:00 2001 From: Ralf Grubenmann Date: Wed, 18 May 2022 08:57:46 +0200 Subject: [PATCH] feat: add git error codes to notebooks response (#1055) * squashme: make error messages properly surface (#1064) Co-authored-by: Ralf Grubenmann Co-authored-by: Tasko Olevski --- git_services/git_services/init/cloner.py | 22 ++++++++++++++------ git_services/git_services/init/errors.py | 26 ++++++++++++------------ renku_notebooks/api/schemas.py | 11 +++++++--- 3 files changed, 37 insertions(+), 22 deletions(-) diff --git a/git_services/git_services/init/cloner.py b/git_services/git_services/init/cloner.py index fde4c3e4d..f8d5129d2 100644 --- a/git_services/git_services/init/cloner.py +++ b/git_services/git_services/init/cloner.py @@ -82,7 +82,17 @@ def _temp_plaintext_credentials(self): # NOTE: Temp credentials MUST be cleaned up on context manager exit logging.info("Cleaning up git credentials after cloning.") credential_loc.unlink(missing_ok=True) - self.cli.git_config("--unset credential.helper") + try: + self.cli.git_config("--unset credential.helper") + except GitCommandError as err: + # INFO: The repo is fully deleted when an error occurs so when the context + # manager exits then this results in an unnecessary error that masks the true + # error, that is why this is ignored. + logging.warning( + "Git plaintext credentials were deleted but could not be " + "unset in the repository's config, most likely because the repository has " + f"been deleted. Detailed error: {err}" + ) def _clone(self, branch): logging.info(f"Cloning branch {branch}") @@ -94,17 +104,17 @@ def _clone(self, branch): self.cli.git_checkout(branch) except GitCommandError as err: if err.returncode != 0 or len(err.stderr) != 0: - if b"no space left on device" in err.stderr: + if "no space left on device" in err.stderr: # INFO: not enough disk space - raise errors.NoDiskSpaceError + raise errors.NoDiskSpaceError from err else: - raise errors.BranchDoesNotExistError + raise errors.BranchDoesNotExistError from err try: logging.info("Dealing with submodules") self.cli.git_submodule("init") self.cli.git_submodule("update") - except GitCommandError: - raise errors.GitSubmoduleError + except GitCommandError as err: + raise errors.GitSubmoduleError from err def _get_autosave_branch(self, session_branch, root_commit_sha): logging.info("Checking for autosaves") diff --git a/git_services/git_services/init/errors.py b/git_services/git_services/init/errors.py index 9cdf5bf2f..5f2068723 100644 --- a/git_services/git_services/init/errors.py +++ b/git_services/git_services/init/errors.py @@ -1,41 +1,41 @@ +import shutil import sys import traceback +from git_services.init.config import config_from_env + class GitCloneGenericError(Exception): """A generic error class that is the parent class of all API errors raised by the git clone module.""" - - default_exit_code = 200 - - def __init__( - self, - exit_code=default_exit_code, - ): - self.exit_code = exit_code + exit_code = 200 class GitServerUnavailableError(GitCloneGenericError): - default_exit_code = 201 + exit_code = 201 class UnexpectedAutosaveFormatError(GitCloneGenericError): - default_exit_code = 202 + exit_code = 202 class NoDiskSpaceError(GitCloneGenericError): - default_exit_code = 203 + exit_code = 203 class BranchDoesNotExistError(GitCloneGenericError): - default_code = 204 + exit_code = 204 class GitSubmoduleError(GitCloneGenericError): - default_code = 205 + exit_code = 205 def handle_exception(exc_type, exc_value, exc_traceback): + # NOTE: To prevent restarts of a failing init container from producing ambiguous errors + # cleanup the repo after a failure so that a restart of the container produces the same error. + config = config_from_env() + shutil.rmtree(config.mount_path, ignore_errors=True) if issubclass(exc_type, GitCloneGenericError): # INFO: The process failed in a specific way that should be distinguished to the user. # The user can take action to correct the failure. diff --git a/renku_notebooks/api/schemas.py b/renku_notebooks/api/schemas.py index d7865fc20..abbfd487d 100644 --- a/renku_notebooks/api/schemas.py +++ b/renku_notebooks/api/schemas.py @@ -310,6 +310,13 @@ def get_user_correctable_message(exit_code): 139: default_server_error_message, # INFO: receiving SIGTERM 143: default_server_error_message, + 200: "Cannot clone repository: Unhandled git error.", + 201: "Cannot clone repository: Git remote server is unavailable. Try again later.", + 202: "Cannot clone repository: " + "Autosave branch name is in an unexpected format and cannot be processed.", + 203: "Cannot clone repository: No disk space left on device.", + 204: "Cannot clone repository: Requested branch doesn't exist on remote.", + 205: "Cannot clone repository: Error fetching submodules.", } return exit_code_msg_xref.get(exit_code, default_server_error_message) @@ -321,9 +328,7 @@ def get_failed_message(failed_containers): for container in failed_containers: exit_code = get_failed_container_exit_code(container) container_name = container.get("name", "Unknown") - if ( - container_name == "git-clone" and exit_code == 128 - ) or container_name == "jupyter-server": + if container_name == "git-clone" or container_name == "jupyter-server": # INFO: The git-clone init container ran out of disk space # or the server container failed user_correctable_message = get_user_correctable_message(exit_code)