diff --git a/isic_cli/cli/__init__.py b/isic_cli/cli/__init__.py index 25ca652..9755024 100644 --- a/isic_cli/cli/__init__.py +++ b/isic_cli/cli/__init__.py @@ -1,5 +1,6 @@ from __future__ import annotations +from contextlib import contextmanager import datetime from http.client import HTTPConnection import logging @@ -7,10 +8,12 @@ import platform import sys import traceback +from typing import TYPE_CHECKING from authlib.integrations.base_client.errors import OAuthError import click -from click import UsageError, get_current_context +from click import Abort, ClickException, UsageError +from click.exceptions import Exit from requests.exceptions import ConnectionError as RequestsConnectionError from requests.exceptions import HTTPError import sentry_sdk @@ -34,6 +37,9 @@ from isic_cli.session import get_session from isic_cli.utils.version import check_for_newer_version, get_version, is_dev_install +if TYPE_CHECKING: + from collections.abc import Iterator + DOMAINS = { "dev": "http://127.0.0.1:8000", "sandbox": "https://api-sandbox.isic-archive.com", @@ -108,7 +114,9 @@ def _sentry_setup(): @click.option("-v", "--verbose", is_flag=True, help="Enable verbose mode.") @click.version_option() @click.pass_context -def cli(ctx, verbose: bool, guest: bool, sandbox: bool, dev: bool, no_version_check: bool): # noqa: FBT001, C901, PLR0913 +def cli(ctx, verbose: bool, guest: bool, sandbox: bool, dev: bool, no_version_check: bool) -> None: # noqa: FBT001, C901, PLR0913 + ctx.with_resource(_report_unexpected_errors(ctx)) + logger.addHandler(logging.StreamHandler(sys.stderr)) logger.setLevel(logging.WARNING) @@ -179,9 +187,15 @@ def cli(ctx, verbose: bool, guest: bool, sandbox: bool, dev: bool, no_version_ch cli.add_command(user_group, name="user") -def main(): +# registered on the root context so it runs while ctx.obj is still available. click tears +# the context down before an exception could reach main. +@contextmanager +def _report_unexpected_errors(ctx: click.Context) -> Iterator[None]: try: - cli() + yield + # click's standalone mode handles these once the context has closed + except (ClickException, Exit, Abort, EOFError, BrokenPipeError): + raise except RequestsConnectionError as e: click.secho( "Unable to connect to the ISIC Archive. Check your network connection and try again.", @@ -201,15 +215,15 @@ def main(): click.echo(traceback.format_exc(), err=True) - ctx = get_current_context(silent=True) env = "-" user = "-" - if ctx and ctx.obj: - env = ctx.obj["env"] + isic_context: IsicContext | None = ctx.obj + if isic_context: + env = isic_context.env - if ctx.obj.user: - user = ctx.obj.user["id"] + if isic_context.user: + user = isic_context.user["id"] set_tag("platform", platform.system()) set_tag("isic-env", env) @@ -228,21 +242,33 @@ def main(): click.echo(f'command: isic {" ".join(sys.argv[1:])}\n', err=True) if is_dev_install(): - return - - send_bug_report = click.prompt( - click.style( - "This is a bug in isic-cli, would you like to send a bug report?", fg="yellow" - ), - type=click.Choice(choices=["y", "n"]), - default="y", - err=True, - show_choices=True, - ) + sys.exit(1) + + # the prompt can't be answered without an interactive terminal (e.g. cron, CI, or piped + # input), so only point to the issue tracker. + send_bug_report = "n" + if sys.stdin.isatty(): + send_bug_report = click.prompt( + click.style( + "This is a bug in isic-cli, would you like to send a bug report?", fg="yellow" + ), + type=click.Choice(choices=["y", "n"]), + default="y", + err=True, + show_choices=True, + ) # this is the only code that actually sends data to sentry, so it's guarded with an opt-in if send_bug_report == "y": capture_exception(e) else: - click.secho("Alternatively you can open an issue below: \n", fg="yellow", err=True) + click.secho( + "You can report this bug by opening an issue below: \n", fg="yellow", err=True + ) click.echo("https://github.com/ImageMarkup/isic-cli/issues/new", err=True) + + sys.exit(1) + + +def main(): + cli() diff --git a/pyproject.toml b/pyproject.toml index 5714454..c2fcb4b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,7 +21,7 @@ classifiers = [ "Programming Language :: Python :: 3.14", ] dependencies = [ - "click>=8.2.0", + "click>=8.3.0", "django-s3-file-field-client>=1.0.0", # We expect girder-cli-oauth-client to drop oob support in the future "girder-cli-oauth-client<1.0.0", diff --git a/tests/test_cli_base.py b/tests/test_cli_base.py index b1aea84..40b8949 100644 --- a/tests/test_cli_base.py +++ b/tests/test_cli_base.py @@ -1,5 +1,7 @@ from __future__ import annotations +import sys + from packaging.version import Version import pytest @@ -37,27 +39,60 @@ def test_new_version( @pytest.mark.parametrize( - ("send_bug_report", "capture_exception_sent"), + ("interactive", "send_bug_report", "capture_exception_sent"), [ - ("y", 1), - ("n", 0), + (True, "y", 1), + (True, "n", 0), + (False, None, 0), ], ) -def test_sentry_error_capture(mocker, send_bug_report, capture_exception_sent): +def test_sentry_error_capture(mocker, capsys, interactive, send_bug_report, capture_exception_sent): # Note: _sentry_setup is always mocked from isic_cli import cli from isic_cli.cli import main - def _exception(): - raise Exception("foo") # noqa: TRY002 - - mocker.patch("isic_cli.cli.cli", side_effect=_exception) - mocker.patch("isic_cli.cli.click.prompt", return_value=send_bug_report) + mocker.patch("isic_cli.cli.collection.get_collections", side_effect=RuntimeError("foo")) + mocker.patch.object( + sys, "argv", ["isic", "--guest", "--no-version-check", "collection", "list"] + ) + prompt = mocker.patch("isic_cli.cli.click.prompt", return_value=send_bug_report) mocker.patch("isic_cli.cli.is_dev_install", return_value=False) + stdin = mocker.patch.object(sys, "stdin") + stdin.isatty.return_value = interactive spy = mocker.spy(cli, "capture_exception") - main() + with pytest.raises(SystemExit) as exc_info: + main() + + assert exc_info.value.code == 1 + assert prompt.called == interactive assert spy.call_count == capture_exception_sent + issue_link_shown = ( + "https://github.com/ImageMarkup/isic-cli/issues/new" in capsys.readouterr().err + ) + assert issue_link_shown == (capture_exception_sent == 0) + + +@pytest.mark.usefixtures("_mock_user") +def test_bug_report_describes_env_and_user(mocker, capsys): + from isic_cli.cli import main + + user = {"id": 1, "email": "fakeuser@email.test"} + mocker.patch("isic_cli.cli.get_users_me", return_value=user) + mocker.patch("isic_cli.cli.collection.get_collections", side_effect=RuntimeError("foo")) + mocker.patch("isic_cli.cli.is_dev_install", return_value=True) + mocker.patch.object( + sys, "argv", ["isic", "--sandbox", "--no-version-check", "collection", "list"] + ) + + with pytest.raises(SystemExit) as exc_info: + main() + + assert exc_info.value.code == 1 + err = capsys.readouterr().err + assert "RuntimeError: foo" in err + assert "env: sandbox" in err + assert f"user: {user['id']}" in err def test_connection_error_is_not_reported_as_bug(mocker, capsys): @@ -66,7 +101,13 @@ def test_connection_error_is_not_reported_as_bug(mocker, capsys): from isic_cli import cli from isic_cli.cli import main - mocker.patch("isic_cli.cli.cli", side_effect=SSLError("EOF occurred in violation of protocol")) + mocker.patch( + "isic_cli.cli.collection.get_collections", + side_effect=SSLError("EOF occurred in violation of protocol"), + ) + mocker.patch.object( + sys, "argv", ["isic", "--guest", "--no-version-check", "collection", "list"] + ) mocker.patch("isic_cli.cli.is_dev_install", return_value=False) prompt = mocker.patch("isic_cli.cli.click.prompt") spy = mocker.spy(cli, "capture_exception") diff --git a/uv.lock b/uv.lock index 0af7255..e89ef38 100644 --- a/uv.lock +++ b/uv.lock @@ -497,7 +497,7 @@ type = [ [package.metadata] requires-dist = [ - { name = "click", specifier = ">=8.2.0" }, + { name = "click", specifier = ">=8.3.0" }, { name = "django-s3-file-field-client", specifier = ">=1.0.0" }, { name = "girder-cli-oauth-client", specifier = "<1.0.0" }, { name = "humanize" },