utils: extract classify_http_error helper, refactor 3 callsites (#38)

hermes_cli/doctor.py

@@ -308,14 +308,24 @@ def _check_devagentic_graph() -> None:
     try:
         with _urllib_request.urlopen(req, timeout=4.0) as resp:
             raw = resp.read().decode("utf-8")
-    except _urllib_error.HTTPError as exc:
-        if exc.code in (401, 403):
+    except (_urllib_error.URLError, OSError, TimeoutError) as exc:
+        # HTTPError is a URLError subclass; classify_http_error (#38)
+        # gives us the one-line auth / 404 / generic / unreachable
+        # dispatch.
+        from utils import (
+            classify_http_error,
+            HTTP_ERROR_AUTH,
+            HTTP_ERROR_NOT_FOUND,
+            HTTP_ERROR_HTTP,
+        )
+        kind = classify_http_error(exc)
+        if kind == HTTP_ERROR_AUTH:
             check_fail(
                 "Devagentic GraphQL: auth failed",
                 "set DEVAGENTIC_API_KEY (any non-empty value when "
                 "devagentic runs in DEVAGENTIC_TRUST_HEADER=1 mode)",
             )
-        elif exc.code == 404:
+        elif kind == HTTP_ERROR_NOT_FOUND:
             check_fail(
                 "Devagentic GraphQL: not found",
                 f"{base}/graphql returned 404. Some devagentic "
@@ -326,17 +336,16 @@ def _check_devagentic_graph() -> None:
                 "'{\"query\":\"{__typename}\"}'` before assuming "
                 "DEVAGENTIC_BASE_URL is wrong.",
             )
-        else:
+        elif kind == HTTP_ERROR_HTTP:
             check_fail(
-                f"Devagentic GraphQL: HTTP {exc.code}",
+                f"Devagentic GraphQL: HTTP {getattr(exc, 'code', '?')}",
                 f"unexpected status from {base}/graphql",
             )
-        return
-    except (_urllib_error.URLError, OSError, TimeoutError) as exc:
-        check_fail(
-            "Devagentic GraphQL: unreachable",
-            f"{base}/graphql — {exc}",
-        )
+        else:
+            check_fail(
+                "Devagentic GraphQL: unreachable",
+                f"{base}/graphql — {exc}",
+            )
         return
 
     try:

plugins/devagentic-canvas/client.py

@@ -120,23 +120,29 @@ def _request(method: str, path: str,
     try:
         with urllib.request.urlopen(req, timeout=timeout) as resp:
             raw = resp.read().decode("utf-8")
-    except urllib.error.HTTPError as exc:
-        if exc.code in (401, 403):
+    except (urllib.error.URLError, OSError, TimeoutError) as exc:
+        # HTTPError is a subclass of URLError; classify_http_error
+        # (#38) handles both via its single dispatch.
+        from utils import (
+            classify_http_error,
+            HTTP_ERROR_AUTH,
+            HTTP_ERROR_NOT_FOUND,
+            HTTP_ERROR_HTTP,
+        )
+        kind = classify_http_error(exc)
+        if kind == HTTP_ERROR_AUTH:
             msg = ("authentication failed — set DEVAGENTIC_API_KEY "
                    "(any non-empty value works when devagentic runs "
                    "in trust-header mode)")
-        elif exc.code == 404:
+        elif kind == HTTP_ERROR_NOT_FOUND:
             msg = f"not found at {url}"
+        elif kind == HTTP_ERROR_HTTP:
+            msg = f"HTTP {getattr(exc, 'code', '?')} from {url}"
         else:
-            msg = f"HTTP {exc.code} from {url}"
+            msg = f"unreachable at {url} ({exc})"
         logger.debug("canvas client: %s %s → %s", method, url, msg)
         _record_error(msg)
         return None
-    except (urllib.error.URLError, OSError, TimeoutError) as exc:
-        msg = f"unreachable at {url} ({exc})"
-        logger.debug("canvas client: %s %s failed: %s", method, url, exc)
-        _record_error(msg)
-        return None
     try:
         payload = json.loads(raw or "null")
     except json.JSONDecodeError as exc:

plugins/devagentic-docs/client.py

@@ -99,21 +99,27 @@ def _post_graphql(query: str, variables: dict,
     try:
         with urllib.request.urlopen(req, timeout=timeout) as resp:
             raw = resp.read().decode("utf-8")
-    except urllib.error.HTTPError as exc:
-        if exc.code in (401, 403):
+    except (urllib.error.URLError, OSError, TimeoutError) as exc:
+        # HTTPError is a URLError subclass; classify_http_error (#38)
+        # handles all four kinds in one dispatch.
+        from utils import (
+            classify_http_error,
+            HTTP_ERROR_AUTH,
+            HTTP_ERROR_NOT_FOUND,
+            HTTP_ERROR_HTTP,
+        )
+        kind = classify_http_error(exc)
+        if kind == HTTP_ERROR_AUTH:
             msg = ("authentication failed — set DEVAGENTIC_API_KEY "
                    "(any non-empty value works when devagentic runs "
                    "in trust-header mode)")
-        elif exc.code == 404:
+        elif kind == HTTP_ERROR_NOT_FOUND:
             msg = f"not found at {url}"
+        elif kind == HTTP_ERROR_HTTP:
+            msg = f"HTTP {getattr(exc, 'code', '?')} from {url}"
         else:
-            msg = f"HTTP {exc.code} from {url}"
-        logger.debug("docs client: %s %s → %s", "POST", url, msg)
-        _record_error(msg)
-        return None
-    except (urllib.error.URLError, OSError, TimeoutError) as exc:
-        msg = f"unreachable at {url} ({exc})"
-        logger.debug("docs client: %s", msg)
+            msg = f"unreachable at {url} ({exc})"
+        logger.debug("docs client: POST %s → %s", url, msg)
         _record_error(msg)
         return None
     try:

tests/test_utils_classify_http_error.py

@@ -0,0 +1,102 @@
+"""Tests for ``utils.classify_http_error`` (#38).
+
+Confirms the urllib-error classifier returns the right kind for
+each of the five categories. Callers (canvas client, docs client,
+hermes_cli.doctor) dispatch on this string to choose their own
+user-facing message text.
+"""
+from __future__ import annotations
+
+import socket
+import urllib.error
+
+import pytest
+
+from utils import (
+    classify_http_error,
+    HTTP_ERROR_AUTH,
+    HTTP_ERROR_NOT_FOUND,
+    HTTP_ERROR_HTTP,
+    HTTP_ERROR_UNREACHABLE,
+    HTTP_ERROR_UNKNOWN,
+)
+
+
+def _http_error(code: int) -> urllib.error.HTTPError:
+    return urllib.error.HTTPError(
+        "http://example.test/", code, f"status {code}", {}, None)
+
+
+# ── auth ──
+
+def test_401_classifies_as_auth():
+    assert classify_http_error(_http_error(401)) == HTTP_ERROR_AUTH
+
+
+def test_403_classifies_as_auth():
+    assert classify_http_error(_http_error(403)) == HTTP_ERROR_AUTH
+
+
+# ── not_found ──
+
+def test_404_classifies_as_not_found():
+    assert classify_http_error(_http_error(404)) == HTTP_ERROR_NOT_FOUND
+
+
+# ── generic http ──
+
+@pytest.mark.parametrize("code", [400, 422, 429, 500, 502, 503, 504])
+def test_other_http_status_classifies_as_http(code):
+    assert classify_http_error(_http_error(code)) == HTTP_ERROR_HTTP
+
+
+# ── unreachable ──
+
+def test_url_error_classifies_as_unreachable():
+    exc = urllib.error.URLError("connection refused")
+    assert classify_http_error(exc) == HTTP_ERROR_UNREACHABLE
+
+
+def test_os_error_classifies_as_unreachable():
+    assert classify_http_error(OSError(111, "Connection refused")) == \
+        HTTP_ERROR_UNREACHABLE
+
+
+def test_timeout_error_classifies_as_unreachable():
+    assert classify_http_error(TimeoutError("operation timed out")) == \
+        HTTP_ERROR_UNREACHABLE
+
+
+def test_socket_timeout_classifies_as_unreachable():
+    """socket.timeout is a subclass of OSError, so it should fall
+    through to the same branch."""
+    assert classify_http_error(socket.timeout("timeout")) == \
+        HTTP_ERROR_UNREACHABLE
+
+
+# ── unknown ──
+
+def test_unrelated_exception_classifies_as_unknown():
+    assert classify_http_error(ValueError("not an http error")) == \
+        HTTP_ERROR_UNKNOWN
+
+
+def test_runtime_error_classifies_as_unknown():
+    """RuntimeError is the kind of exception SDKs sometimes raise
+    in lieu of HTTPError. It SHOULD NOT be classified as auth or
+    network — leaving it as "unknown" tells the caller "this isn't
+    one of the standard urllib failure modes" so they can fall
+    through to message-substring heuristics if they need to."""
+    assert classify_http_error(RuntimeError("401 Unauthorized")) == \
+        HTTP_ERROR_UNKNOWN
+
+
+# ── HTTPError-is-URLError subclass quirk ──
+
+def test_http_error_takes_precedence_over_url_error_branch():
+    """urllib.error.HTTPError inherits from URLError. The classifier
+    must check HTTPError FIRST so a 401 isn't misread as just
+    'unreachable'."""
+    exc = _http_error(401)
+    assert isinstance(exc, urllib.error.URLError)
+    assert classify_http_error(exc) == HTTP_ERROR_AUTH

utils.py

@@ -359,3 +359,50 @@ def base_url_host_matches(base_url: str, domain: str) -> bool:
     if not domain:
         return False
     return hostname == domain or hostname.endswith("." + domain)
+
+
+# ---------------------------------------------------------------------------
+# urllib error classification (#38)
+#
+# Several modules dispatch on urllib HTTPError / URLError / OSError /
+# TimeoutError to decide whether a failure is auth / not-found / network /
+# other. Each callsite then formats its own user-facing message. Extracting
+# the classification keeps the message text local while consolidating the
+# dispatch rules.
+# ---------------------------------------------------------------------------
+
+# Kind constants — callers can compare against these without importing
+# urllib directly.
+HTTP_ERROR_AUTH = "auth"
+HTTP_ERROR_NOT_FOUND = "not_found"
+HTTP_ERROR_HTTP = "http"
+HTTP_ERROR_UNREACHABLE = "unreachable"
+HTTP_ERROR_UNKNOWN = "unknown"
+
+
+def classify_http_error(exc: BaseException) -> str:
+    """Classify a urllib-style exception into one of:
+
+      * ``"auth"``         — HTTP 401 / 403
+      * ``"not_found"``    — HTTP 404
+      * ``"http"``         — any other HTTP status code from HTTPError
+      * ``"unreachable"``  — URLError / OSError / TimeoutError (DNS,
+                              connection refused, timeout, etc.)
+      * ``"unknown"``      — anything else (callers should treat this as
+                              a programmer error, not an operational one)
+
+    Callers format the user-facing message themselves — this helper only
+    answers "which class of failure is this".
+    """
+    import urllib.error as _urllib_error
+
+    if isinstance(exc, _urllib_error.HTTPError):
+        code = getattr(exc, "code", None)
+        if code in (401, 403):
+            return HTTP_ERROR_AUTH
+        if code == 404:
+            return HTTP_ERROR_NOT_FOUND
+        return HTTP_ERROR_HTTP
+    if isinstance(exc, (_urllib_error.URLError, OSError, TimeoutError)):
+        return HTTP_ERROR_UNREACHABLE
+    return HTTP_ERROR_UNKNOWN