Fragile exception parsing in `exec_git_command` should be replaced with more robust error handling mechanisms in `github_utils.py`.

Certainly! One notable issue in this existing program lies in the exception handling mechanism in the github.py module, specifically within the exec_git_command function in github_utils.py. The code attempts to handle various Git-related exceptions by parsing the error messages, which is usually a fragile and error-prone approach. Here are the details:

Specific Issue

File: `services/github/github_utils.py`

Function: `exec_git_command`

def exec_git_command(
        repo: str,
        command: list[str],
        capture_output: bool = False) -> subprocess.CompletedProcess:
    ...
    try:
        ...
    except subprocess.CalledProcessError as err:
        shorted_commands = " ".join(command)[:50]
        log(
            f"Command {shorted_commands} failed with error ({err.returncode}): {err}",
            level="exception",
        )
        exception, error_message = exceptions.parse_exception(err)
        raise exception(error_message) from err

Problem Description

Fragile Exception Parsing: The parse_exception function relies on checking for specific substrings within error messages to deduce the type of exception to raise. This approach is brittle because it can easily break if the error messages change slightly, or if different locales or versions produce slightly varied messages.

Proposed Solution

The ideal solution would involve a more robust way to handle errors, possibly using specific exception classes provided by subprocess or another tool. For instance, enhancing logging mechanisms and categorizing exceptions by error codes rather than strings could significantly improve the reliability and maintainability of the code. Here is an example implementation:

Refactored Code Example

def exec_git_command(
        repo: str,
        command: list[str],
        capture_output: bool = False) -> subprocess.CompletedProcess:
    try:
        complete_process = subprocess.run(
            command,
            stdout=subprocess.PIPE if capture_output else None,
            stderr=subprocess.PIPE if capture_output else None,
            cwd=os.path.join(DEFAULT_PATH, repo),
            check=True)
        return complete_process
    except subprocess.CalledProcessError as err:
        error_message = f"Command '{' '.join(command)}' failed with return code {err.returncode}"
        log(error_message, level="error")
        # Use pre-defined error classes or map error codes to exception classes instead
        raise exceptions.CommandExecutionException(error_message) from err

Summary:

The current handling of exceptions by parsing specific strings in error messages is not reliable and introduces potential points of failure. Enhancing the error handling approach to use more structured and resilient methods can improve the overall robustness of the program.

tawada / grass-grower