From e057185c2dfca680bab4d30efd89503f216aaa5f Mon Sep 17 00:00:00 2001
From: Ike Hecht <tosfos@yahoo.com>
Date: Sun, 1 Mar 2026 19:24:49 -0500
Subject: [PATCH] fix(cli): graceful handling for mwclient/requests exceptions

Catch MwClientError (LoginError, APIError, etc.) and RequestException
(ConnectionError, Timeout) from build_site(), method(), maybe_convert_markdown()
(site.parse), and iterator iteration. Print a clean message to stderr and
exit 1 instead of dumping a raw traceback.

- Add _handle_runtime_error() helper; optionally import requests.exceptions
- Wrap maybe_convert_markdown in same try/except as method() so parse() failures are handled
- Tests: APIError, ConnectionError, and iterator mid-stream raise

Made-with: Cursor
---
 mwclient_cli/cli.py | 46 +++++++++++++++++++++++++++++++++++++--------
 tests/test_cli.py   | 43 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 8 deletions(-)

diff --git a/mwclient_cli/cli.py b/mwclient_cli/cli.py
index 584a37e..ac64089 100644
--- a/mwclient_cli/cli.py
+++ b/mwclient_cli/cli.py
@@ -13,6 +13,26 @@
 from typing import Any
 
 import html2text
+import mwclient.errors as mwclient_errors
+
+try:
+    import requests.exceptions as requests_exceptions
+except ImportError:
+    requests_exceptions = None  # type: ignore[assignment]
+
+
+def _handle_runtime_error(e: Exception) -> int:
+    """Print a graceful CLI message for mwclient/requests errors; return exit code 1."""
+    if isinstance(e, mwclient_errors.MwClientError):
+        print(str(e), file=sys.stderr)
+        return 1
+    if requests_exceptions is not None and isinstance(
+        e, requests_exceptions.RequestException
+    ):
+        print(str(e), file=sys.stderr)
+        return 1
+    print(f"Error: {e}", file=sys.stderr)
+    return 1
 
 _ENTITY_LOCATIONS = {
     "site": ("mwclient.client", "Site"),
@@ -361,22 +381,32 @@ def run(argv: list[str] | None = None) -> int:
     except ValueError as exc:
         parser.error(str(exc))
 
-    target = build_target(build_site(args), args)
+    try:
+        target = build_target(build_site(args), args)
+    except Exception as e:
+        return _handle_runtime_error(e)
+
     methods = list_public_methods(args.command)
     if args.method not in methods:
         parser.error(f"Unknown method {args.command}.{args.method}")
     method = getattr(target, args.method)
 
-    result = method(*positionals, **kwargs)
-    result = maybe_convert_markdown(args, target, result)
+    try:
+        result = method(*positionals, **kwargs)
+        result = maybe_convert_markdown(args, target, result)
+    except Exception as e:
+        return _handle_runtime_error(e)
     if isinstance(result, Iterator):
         max_items = args.max_items
         count = 0
-        for item in result:
-            print_json(item, args.indent)
-            count += 1
-            if max_items is not None and count >= max_items:
-                break
+        try:
+            for item in result:
+                print_json(item, args.indent)
+                count += 1
+                if max_items is not None and count >= max_items:
+                    break
+        except Exception as e:
+            return _handle_runtime_error(e)
         return 0
 
     if args.stream and isinstance(result, (list, tuple)):
diff --git a/tests/test_cli.py b/tests/test_cli.py
index 34651e3..68b4031 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -3,6 +3,8 @@
 
 import pytest
 
+import mwclient.errors as mwclient_errors
+
 from mwclient_cli import cli
 
 
@@ -58,6 +60,47 @@ def test_parse_keyword_args_rejects_invalid():
         cli.parse_keyword_args(["broken"])
 
 
+def test_mwclient_error_prints_gracefully_and_returns_1(capsys):
+    """Uncaught mwclient/requests errors become a clean stderr message and exit 1."""
+    with patch.object(cli, "build_site", side_effect=mwclient_errors.APIError("badparam", "Invalid parameter 'x'", None)):
+        rc = cli.run(["--host", "example.org", "site", "ping"])
+    assert rc == 1
+    err = capsys.readouterr().err
+    assert "Invalid parameter" in err or "badparam" in err
+
+
+def test_requests_connection_error_prints_gracefully(capsys):
+    """requests.exceptions (e.g. ConnectionError) get a clean stderr message and exit 1."""
+    import requests.exceptions
+
+    with patch.object(
+        cli, "build_site", side_effect=requests.exceptions.ConnectionError("Connection refused")
+    ):
+        rc = cli.run(["--host", "example.org", "site", "ping"])
+    assert rc == 1
+    assert "Connection refused" in capsys.readouterr().err
+
+
+def test_iterator_error_mid_stream(capsys):
+    """Generator that raises mid-iteration is caught and reported gracefully."""
+
+    def failing_generator(self):
+        yield {"n": 0}
+        raise mwclient_errors.APIError("internal", "Server error", None)
+
+    DummyPage.failing = failing_generator
+    try:
+        with patch.object(cli, "build_site", return_value=DummySite()), patch.object(
+            cli, "list_public_methods", return_value={"failing": failing_generator}
+        ):
+            rc = cli.run(["--host", "example.org", "page", "Test", "failing"])
+        assert rc == 1
+        err = capsys.readouterr().err
+        assert "Server error" in err or "internal" in err
+    finally:
+        del DummyPage.failing
+
+
 def test_site_method_call_prints_json(capsys):
     with patch.object(cli, "build_site", return_value=DummySite()), patch.object(
         cli, "list_public_methods", return_value={"ping": DummySite.ping}