Various bug report fixes

isic_cli/cli/__init__.py

@@ -1,16 +1,19 @@
 from __future__ import annotations
 
+from contextlib import contextmanager
 import datetime
 from http.client import HTTPConnection
 import logging
 import os
 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()

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",

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")