From c35f0a93613048dc9733c028577409bf527d4a97 Mon Sep 17 00:00:00 2001 From: angus-yxz <138585512+angus-yxz@users.noreply.github.com> Date: Thu, 13 Aug 2026 05:14:11 +0800 Subject: [PATCH 1/3] added exception handling for latest release checking and tests --- app/cli.py | 28 +++++++++++++++++++++------- tests/e2e/test_version.py | 17 +++++++++++++++++ 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/app/cli.py b/app/cli.py index 941e25c..e0ac230 100644 --- a/app/cli.py +++ b/app/cli.py @@ -23,6 +23,9 @@ def invoke(self, ctx: click.Context) -> None: CONTEXT_SETTINGS = {"max_content_width": 120} +# Bound the release check so the CLI cannot hang on an unresponsive network +LATEST_RELEASE_TIMEOUT_SECONDS = 5 + @click.group( cls=LoggingGroup, @@ -39,14 +42,25 @@ def cli(ctx: click.Context, verbose: bool) -> None: current_version = Version.parse_version_string(__version__) ctx.obj[CliContextKey.VERSION] = current_version - latest_version = ( - requests.get( - "https://github.com/git-mastery/app/releases/latest", allow_redirects=False + + latest_version = None + try: + response = requests.get( + "https://github.com/git-mastery/app/releases/latest", + allow_redirects=False, + timeout=LATEST_RELEASE_TIMEOUT_SECONDS, ) - .headers["Location"] - .rsplit("/", 1)[-1] - ) - if current_version.is_behind(Version.parse_version_string(latest_version)): + # GitHub redirects to the tag of the latest release; without the redirect + # there is no version to compare against + location = response.headers.get("Location") + if location is not None: + latest_version = Version.parse_version_string(location.rsplit("/", 1)[-1]) + except (requests.exceptions.RequestException, ValueError): + latest_version = None + + if latest_version is None: + warn("Unable to verify the latest version release") + elif current_version.is_behind(latest_version): warn( click.style( f"Your version of Git-Mastery app {current_version} is behind the latest version {latest_version}.", diff --git a/tests/e2e/test_version.py b/tests/e2e/test_version.py index bc9d638..11c40d4 100644 --- a/tests/e2e/test_version.py +++ b/tests/e2e/test_version.py @@ -7,3 +7,20 @@ def test_version(runner: BinaryRunner) -> None: res.assert_success() res.assert_stdout_contains("Git-Mastery app is") res.assert_stdout_matches(r"v\d+\.\d+\.\d+") + + +def test_version_unreachable_release_check(runner: BinaryRunner) -> None: + """Commands still succeed when the latest release cannot be fetched.""" + # Route the release check through a closed port so it cannot connect. + # NO_PROXY is cleared because an inherited value would bypass the proxy. + res = runner.run( + ["version"], + env={ + "HTTP_PROXY": "http://127.0.0.1:1", + "HTTPS_PROXY": "http://127.0.0.1:1", + "NO_PROXY": "", + }, + ) + res.assert_success() + res.assert_stdout_contains("Unable to verify the latest version release") + res.assert_stdout_contains("Git-Mastery app is") From 0ed625d930f2a416e6cccc162825627f792d5bab Mon Sep 17 00:00:00 2001 From: angus-yxz <138585512+angus-yxz@users.noreply.github.com> Date: Fri, 14 Aug 2026 17:56:20 +0800 Subject: [PATCH 2/3] Improved error logging, moved log into exception branch --- app/cli.py | 15 +++++++++------ tests/e2e/test_version.py | 17 ----------------- 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/app/cli.py b/app/cli.py index e0ac230..7eafd3f 100644 --- a/app/cli.py +++ b/app/cli.py @@ -53,14 +53,17 @@ def cli(ctx: click.Context, verbose: bool) -> None: # GitHub redirects to the tag of the latest release; without the redirect # there is no version to compare against location = response.headers.get("Location") - if location is not None: + if location is None: + warn( + "Unable to verify the latest version release: no redirect to the latest " + f"release tag (status {response.status_code})" + ) + else: latest_version = Version.parse_version_string(location.rsplit("/", 1)[-1]) - except (requests.exceptions.RequestException, ValueError): - latest_version = None + except (requests.exceptions.RequestException, ValueError) as e: + warn(f"Unable to verify the latest version release: {e}") - if latest_version is None: - warn("Unable to verify the latest version release") - elif current_version.is_behind(latest_version): + if latest_version is not None and current_version.is_behind(latest_version): warn( click.style( f"Your version of Git-Mastery app {current_version} is behind the latest version {latest_version}.", diff --git a/tests/e2e/test_version.py b/tests/e2e/test_version.py index 11c40d4..bc9d638 100644 --- a/tests/e2e/test_version.py +++ b/tests/e2e/test_version.py @@ -7,20 +7,3 @@ def test_version(runner: BinaryRunner) -> None: res.assert_success() res.assert_stdout_contains("Git-Mastery app is") res.assert_stdout_matches(r"v\d+\.\d+\.\d+") - - -def test_version_unreachable_release_check(runner: BinaryRunner) -> None: - """Commands still succeed when the latest release cannot be fetched.""" - # Route the release check through a closed port so it cannot connect. - # NO_PROXY is cleared because an inherited value would bypass the proxy. - res = runner.run( - ["version"], - env={ - "HTTP_PROXY": "http://127.0.0.1:1", - "HTTPS_PROXY": "http://127.0.0.1:1", - "NO_PROXY": "", - }, - ) - res.assert_success() - res.assert_stdout_contains("Unable to verify the latest version release") - res.assert_stdout_contains("Git-Mastery app is") From c952d95c747cb825e3d18d4788edd903736bcbc4 Mon Sep 17 00:00:00 2001 From: jovnc <95868357+jovnc@users.noreply.github.com> Date: Sat, 15 Aug 2026 22:59:23 +0800 Subject: [PATCH 3/3] Move fetch latest version logic to version.py --- app/cli.py | 29 +++++------------------------ app/utils/version.py | 24 +++++++++++++++++++++++- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/app/cli.py b/app/cli.py index 7eafd3f..4b10816 100644 --- a/app/cli.py +++ b/app/cli.py @@ -2,7 +2,6 @@ import sys import click -import requests from click_aliases import ClickAliasedGroup from app.aliases import COMMAND_ALIASES @@ -10,7 +9,7 @@ from app.commands.repl import repl from app.commands.version import version from app.utils.click import ClickColor, CliContextKey, warn -from app.utils.version import Version +from app.utils.version import Version, fetch_latest_release_version from app.version import __version__ @@ -23,9 +22,6 @@ def invoke(self, ctx: click.Context) -> None: CONTEXT_SETTINGS = {"max_content_width": 120} -# Bound the release check so the CLI cannot hang on an unresponsive network -LATEST_RELEASE_TIMEOUT_SECONDS = 5 - @click.group( cls=LoggingGroup, @@ -43,25 +39,10 @@ def cli(ctx: click.Context, verbose: bool) -> None: current_version = Version.parse_version_string(__version__) ctx.obj[CliContextKey.VERSION] = current_version - latest_version = None - try: - response = requests.get( - "https://github.com/git-mastery/app/releases/latest", - allow_redirects=False, - timeout=LATEST_RELEASE_TIMEOUT_SECONDS, - ) - # GitHub redirects to the tag of the latest release; without the redirect - # there is no version to compare against - location = response.headers.get("Location") - if location is None: - warn( - "Unable to verify the latest version release: no redirect to the latest " - f"release tag (status {response.status_code})" - ) - else: - latest_version = Version.parse_version_string(location.rsplit("/", 1)[-1]) - except (requests.exceptions.RequestException, ValueError) as e: - warn(f"Unable to verify the latest version release: {e}") + # Latest version checking is a soft dependency, should not fail operation + latest_version, latest_version_error = fetch_latest_release_version() + if latest_version_error is not None: + warn(latest_version_error) if latest_version is not None and current_version.is_behind(latest_version): warn( diff --git a/app/utils/version.py b/app/utils/version.py index 600a7ee..de78a83 100644 --- a/app/utils/version.py +++ b/app/utils/version.py @@ -1,6 +1,11 @@ from dataclasses import dataclass from typing import Optional +import requests + +LATEST_RELEASE_TIMEOUT_SECONDS = 5 +LATEST_RELEASE_URL = "https://github.com/git-mastery/app/releases/latest" + @dataclass class Version: @@ -24,7 +29,9 @@ def parse_version_string(version: str) -> "Version": def parse(version: str) -> "Version": """Parse a plain version string (e.g., '1.2.3').""" parts = version.split(".") - if ("beta" in version and len(parts) != 4) or ("beta" not in version and len(parts) != 3): + if ("beta" in version and len(parts) != 4) or ( + "beta" not in version and len(parts) != 3 + ): raise ValueError( f"Invalid version string (expected 'MAJOR.MINOR.PATCH[-beta.PRERELEASE]'): {version!r}" ) @@ -57,3 +64,18 @@ def __repr__(self) -> str: if self.prerelease is not None: return f"v{self.major}.{self.minor}.{self.patch}-beta.{self.prerelease}" return f"v{self.major}.{self.minor}.{self.patch}" + + +def fetch_latest_release_version() -> tuple[Optional["Version"], Optional[str]]: + try: + response = requests.get( + LATEST_RELEASE_URL, + allow_redirects=False, + timeout=LATEST_RELEASE_TIMEOUT_SECONDS, + ) + location = response.headers.get("Location") + if location is None: + return None, "Unable to verify the latest version release" + return Version.parse_version_string(location.rsplit("/", 1)[-1]), None + except (requests.exceptions.RequestException, ValueError) as e: + return None, f"Unable to verify the latest version release: {e}"