Improve clone error messages (#588)

* feat: show a nicer error message if git clone fails

* feat: check if host added to known hosts if clone fails

* chore: mypy and formatting fixes

* chore: revert error update to fix failing tests

* chore: revert log level change

* chore: update changelog

* chore: fix a typo

CHANGELOG.md

@@ -11,12 +11,15 @@ and this project adheres to [Semantic Versioning][semver].
 
 ### Added
 
+- Improved Git error messaging to provide clearer troubleshooting instructions for remote operations [(588)]
 
 ### Changed
 
 
 ### Fixed
 
+[588]: https://github.com/openlawlibrary/taf/pull/588
+
 
 ## [0.34.0]
 

taf/exceptions.py

@@ -12,11 +12,16 @@ def __str__(self):
             return super().__str__()
 
 
-class CloneRepoException(TAFError):
-    def __init__(self, repo):
-        self.message = (
-            f"Cannot clone {repo.name} from any of the following URLs: {repo.urls}"
-        )
+class GitAccessDeniedException(TAFError):
+    def __init__(self, repo, operation, message=None):
+        self.message = f"Cannot {operation} {repo.name} from any of the following URLs: {repo.urls}"
+        if message is not None:
+            self.message = f"{self.message}\n{message}"
+
+
+class CloneRepoException(GitAccessDeniedException):
+    def __init__(self, repo, operation="clone", message=None):
+        super().__init__(repo, operation, message)
 
 
 class CommandValidationError(TAFError):

taf/git.py

@@ -13,8 +13,10 @@
 from functools import partial, reduce
 from pathlib import Path
 
+import requests  # type: ignore
 import taf.settings as settings
 from taf.exceptions import (
+    GitAccessDeniedException,
     NoRemoteError,
     NothingToCommitError,
     PushFailedError,
@@ -729,18 +731,18 @@ def clone(
                     url,
                     joined_params,
                     log_success_msg=f"successfully cloned from {url}",
-                    log_error_msg=f"cannot clone from url {url}",
                     reraise_error=True,
+                    error_if_not_exists=False,
                 )
             except GitError as e:
-                self._log_info(f"could not clone from {url} due to {e}")
+                self._log_debug(f"could not clone from {url} due to {e}")
             else:
                 self._log_info(f"successfully cloned from {url}")
                 cloned = True
                 break
 
         if not cloned:
-            raise CloneRepoException(self)
+            self.raise_git_access_error(CloneRepoException)
 
         if self.default_branch is None:
             self.default_branch = self._determine_default_branch()
@@ -1117,12 +1119,17 @@ def fetch(
         branch: Optional[str] = None,
         remote: Optional[str] = "origin",
     ) -> None:
-        if fetch_all:
-            self._git("fetch --all", log_error=True)
-        else:
-            if branch is None:
-                branch = ""
-            self._git("fetch {} {}", remote, branch, log_error=True)
+        try:
+            if fetch_all:
+                self._git("fetch --all", log_error=True, reraise_error=True)
+            else:
+                if branch is None:
+                    branch = ""
+                self._git(
+                    "fetch {} {}", remote, branch, log_error=True, reraise_error=True
+                )
+        except GitError:
+            self.raise_git_access_error(operation="fetch")
 
     def fetch_from_disk(self, local_repo_path, branches):
 
@@ -1439,7 +1446,10 @@ def merge_branch(
 
     def pull(self):
         """Pull current branch"""
-        self._git("pull", log_error=True)
+        try:
+            self._git("pull", log_error=True, reraise_error=True)
+        except GitError:
+            self.raise_git_access_error(operation="pull")
 
     def push(
         self,
@@ -1478,12 +1488,35 @@ def push(
         except GitError as e:
             raise PushFailedError(self, message=f"Push operation failed: {e}")
 
+    def raise_git_access_error(
+        self, error_cls=GitAccessDeniedException, operation=None
+    ):
+        hosts = {extract_hostname(url) for url in self.urls}
+        unknown_hosts = [host for host in hosts if not is_host_known(host)]
+        if len(unknown_hosts):
+            message = _no_hosts_error_format.format(hostname=",".join(unknown_hosts))
+            raise error_cls(self, operation=operation, message=message)
+        repo_exists = any(repository_exists(url) for url in self.urls)
+        if repo_exists:
+            uses_ssh = any(url.startswith("git@") for url in self.urls)
+            if uses_ssh:
+                raise error_cls(
+                    self, operation=operation, message=_clone_or_pull_error_message
+                )
+            else:
+                raise error_cls(
+                    self,
+                    operation=operation,
+                    message=_clone_or_pull_error_message_no_ssh,
+                )
+        raise error_cls(self)
+
     def remove_remote(self, remote_name: str) -> None:
         try:
             self._git("remote remove {}", remote_name)
         except GitError as e:
             if "No such remote" not in str(e):
-                self._git("remote rename {remote_name} local")
+                self._git(f"remote rename {remote_name} local")
                 self._log_warning(
                     f"Could not remove remote {remote_name}. It was renamed to 'local'. Remove it manually"
                 )
@@ -1775,6 +1808,39 @@ def _find_url(path, url):
         return urls
 
 
+_clone_or_pull_error_message = (
+    "The remote repository exists, so this is probably due to the lack of privileges or SSH key issues. "
+    "Here are some steps you can take to resolve common issues:\n\n"
+    "1. Check Repository Access:\n"
+    "   - If the repository is private, verify that your account has been granted access.\n\n"
+    "2. Check SSH Key Configuration:\n"
+    "   - Ensure your SSH key is correctly added to your Git hosting account (GitHub, GitLab, Bitbucket, etc.).\n"
+    "   - Verify that your SSH key does not require a passphrase, or use an SSH agent to manage the passphrase.\n\n"
+    "3. Using an SSH Agent:\n"
+    "   - If your SSH key is passphrase-protected, start the ssh-agent in the background:\n"
+    "       eval $(ssh-agent -s)\n"
+    "   - Add your SSH key to the ssh-agent:\n"
+    "       ssh-add /path/to/your/private/key\n"
+    "4. If Problems Persist:\n"
+    "   - If you are using Windows and encounter issues, consider running these commands in Bash or another Unix-like shell available on Windows, such as Git Bash or WSL.\n"
+    "   - Try using a different SSH key that does not require a passphrase.\n"
+)
+
+
+_clone_or_pull_error_message_no_ssh = (
+    "The remote repository exists, so this issue is probably due to lack of privileges.\n"
+    "Verify that you have access to the repository if it is private, and verify your HTTPS configuration.\n"
+    "Consider switching to SSH for potentially enhanced security and easier handling of credentials."
+)
+
+
+_no_hosts_error_format = (
+    "Error: The host(s) '{hostname}' not recognized."
+    "To add a host to your known_hosts file, manually connect via SSH using the command:\n"
+    "ssh -T git@{hostname}\n"
+    "Accept the host key to proceed.\n"
+)
+
 _remote_branch_re = re.compile(r"^(refs/heads/)")
 
 _local_branch_re = re.compile(r"^(origin/)")
@@ -1795,3 +1861,54 @@ def _find_url(path, url):
 _ssh_url = re.compile(
     r"((git|ssh|http(s)?)|(git@[\w\.]+))(:(//)?)([\w\.@\:/\-~]+)(\.git)?(/)?"
 )
+
+
+def extract_hostname(url):
+    """Extract the hostname from a Git URL, which can be either SSH or HTTP(S)."""
+    if url.startswith("git@"):
+        # Handle SSH URL
+        match = re.match(r"git@(.*?):", url)
+    else:
+        # Handle HTTP(S) URL
+        match = re.match(r"https?://([^/]+)/", url)
+
+    if match:
+        return match.group(1)
+    else:
+        return None
+
+
+def is_host_known(hostname):
+    """Check if the given hostname is in the known_hosts file."""
+    known_hosts_path = os.path.expanduser("~/.ssh/known_hosts")
+    try:
+        with open(known_hosts_path, "r") as file:
+            for line in file:
+                if line.startswith(hostname) or hostname in line.split():
+                    return True
+        return False
+    except FileNotFoundError:
+        print("Known hosts file not found.")
+        return False
+    except Exception as e:
+        print(f"An error occurred: {str(e)}")
+        return False
+
+
+def repository_exists(url):
+    """Check if the repository URL exists by making a HEAD request to the URL."""
+    # Convert SSH URL to HTTP URL
+    if url.startswith("git@"):
+        # General pattern: git@HOST:USER/REPO
+        http_url = re.sub(r"git@(.*?):(.*?)/(.*)\.git", r"https://\1/\2/\3", url)
+    elif url.startswith("git://"):
+        http_url = url.replace("git://", "https://").replace(".git", "")
+    else:
+        http_url = url
+
+    try:
+        response = requests.head(http_url)
+        return response.status_code == 200
+    except requests.RequestException as e:
+        print(f"Error checking repository URL: {e}")
+        return False

taf/tools/repo/__init__.py

@@ -48,7 +48,6 @@ def _call_updater(config, format_output):
             taf_logger.error(json.dumps(error_data))
             sys.exit(1)
         else:
-            taf_logger.error(str(e))
             sys.exit(1)
 
 

taf/utils.py

@@ -24,10 +24,6 @@
 from taf.log import taf_logger
 import taf.settings
 from taf.exceptions import PINMissmatchError
-
-# TODO: Remove legacy imports
-# from taf.log import taf_logger
-
 from typing import List, Optional, Tuple, Dict
 from securesystemslib.hash import digest_fileobject
 from securesystemslib.storage import FilesystemBackend, StorageBackendInterface
@@ -418,8 +414,6 @@ def ensure_pre_push_hook(auth_repo_path: Path) -> bool:
     taf_logger.info("Pre-push hook updated successfully.")
     return True
 
-    return True
-
 
 class timed_run:
     """Decorator to let us capture the elapsed time and optionally print a timer and start/end