fix(workflow): surface unparseable cloud 200 body as an error instead of empty/success (BE-3334)

comfy_cli/command/workflow.py

@@ -438,6 +438,12 @@ class _ResponseTooLarge(Exception):
     """A ``/userdata`` response exceeded ``_USERDATA_MAX_BYTES`` — refuse to truncate."""
 
 
+class _ResponseUnparseable(Exception):
+    """A non-empty 200 body could not be decoded as JSON — surface it as a loud
+    error instead of a misleading empty/success result (an empty body is still a
+    legitimate ``None`` and is *not* this)."""
+
+
 # Map the cloud ``--sort`` fields onto local FileInfo keys (client-side sort;
 # ComfyUI's /userdata listing has no server-side sort/limit/filter).
 _LOCAL_SORT_KEYS = {"create_time": "created", "update_time": "modified", "name": "path"}
@@ -618,17 +624,38 @@ def _http_request(
     if not raw:
         return status, None
     try:
-        return status, json.loads(raw)
-    except json.JSONDecodeError:
-        return status, None
+        return status, json.loads(raw.decode("utf-8"))
+    except (ValueError, RecursionError) as e:
+        # A non-empty body that won't decode as JSON is a *malformed* response, not
+        # "no data": returning ``None`` here would let callers report an empty list
+        # or a null id as success. Raise instead so it surfaces as a loud, mapped
+        # error. Decode as UTF-8 *explicitly* first: handed raw bytes, ``json.loads``
+        # auto-detects UTF-16/32 (RFC 4627) and would silently accept a non-UTF-8 body
+        # the contract treats as malformed. Non-UTF-8 bytes -> ``UnicodeDecodeError``;
+        # valid-UTF-8 non-JSON text -> ``JSONDecodeError``; both subclass ``ValueError``.
+        # Catch the ``ValueError`` base to also map the parser's *other* rejections that
+        # aren't ``JSONDecodeError`` — e.g. a JSON integer past CPython's 4300-digit
+        # int/str limit raises a bare ``ValueError`` — plus ``RecursionError`` from
+        # pathologically nested input (the 64 MiB cap permits deep nesting). Otherwise
+        # those escape the mapping and crash the CLI with a raw traceback.
+        raise _ResponseUnparseable() from e
 
 
 def _handle_cloud_http_error(renderer, e, *, operation: str, workflow_id: str | None = None) -> typer.Exit:
     """Map HTTP failures to envelope codes. Returns an Exit to ``raise from``."""
     import urllib.error
 
-    if isinstance(e, urllib.error.HTTPError):
-        body = (e.read() or b"")[:1000].decode("utf-8", "replace")
+    if isinstance(e, _ResponseUnparseable):
+        renderer.error(
+            code="workflow_unparseable",
+            message=f"cloud returned a non-empty but unparseable (non-JSON) response during {operation}",
+            hint="the server sent a malformed body; retry, and report it if it persists",
+            details={"operation": operation, "workflow_id": workflow_id},
+        )
+    elif isinstance(e, urllib.error.HTTPError):
+        # Cap the read itself — ``[:1000]`` after a full ``read()`` would still pull an
+        # arbitrarily large (or malicious) error page into memory before slicing.
+        body = (e.read(1000) or b"").decode("utf-8", "replace")
         if e.code == 404:
             renderer.error(
                 code="workflow_not_found",
@@ -935,9 +962,22 @@ def list_cmd(
 
     try:
         _, body = _http_request(url, target)
-    except (urllib.error.HTTPError, urllib.error.URLError, OSError) as e:
+    except (urllib.error.HTTPError, urllib.error.URLError, OSError, _ResponseUnparseable) as e:
         raise _handle_cloud_http_error(renderer, e, operation="list") from e
 
+    # A non-empty body decodes here only if it was valid JSON; guard the shape the
+    # way ``get``/``save`` do. An empty body is a legitimate ``None`` (→ no rows), but
+    # a valid-JSON *non-dict* 200 (an array like ``[1, 2, 3]`` or a scalar) is malformed:
+    # ``(body).get("data")`` would raise a raw ``AttributeError``, and coercing it to an
+    # empty list would masquerade the malformed shape as a genuinely-empty listing.
+    if body is not None and not isinstance(body, dict):
+        renderer.error(
+            code="cloud_http_error",
+            message="unexpected response shape from /api/workflows (expected a JSON object)",
+            details={"got_type": type(body).__name__},
+        )
+        raise typer.Exit(code=1)
+
     rows = (body or {}).get("data") or []
     payload = {
         "count": len(rows),
@@ -1006,7 +1046,7 @@ def get_cmd(
     url = target.url("workflows", _up.quote(workflow_id, safe=""), "content")
     try:
         _, body = _http_request(url, target)
-    except (urllib.error.HTTPError, urllib.error.URLError, OSError) as e:
+    except (urllib.error.HTTPError, urllib.error.URLError, OSError, _ResponseUnparseable) as e:
         raise _handle_cloud_http_error(renderer, e, operation="get", workflow_id=workflow_id) from e
 
     if not isinstance(body, dict) or "workflow_json" not in body:
@@ -1101,7 +1141,7 @@ def save_cmd(
     url = target.url("workflows")
     try:
         _, resp = _http_request(url, target, method="POST", body=body)
-    except (urllib.error.HTTPError, urllib.error.URLError, OSError) as e:
+    except (urllib.error.HTTPError, urllib.error.URLError, OSError, _ResponseUnparseable) as e:
         raise _handle_cloud_http_error(renderer, e, operation="save") from e
 
     workflow_id = (resp or {}).get("id") if isinstance(resp, dict) else None
@@ -1137,7 +1177,7 @@ def delete_cmd(
     url = target.url("workflows", _up.quote(workflow_id, safe=""))
     try:
         _, _body = _http_request(url, target, method="DELETE")
-    except (urllib.error.HTTPError, urllib.error.URLError, OSError) as e:
+    except (urllib.error.HTTPError, urllib.error.URLError, OSError, _ResponseUnparseable) as e:
         raise _handle_cloud_http_error(renderer, e, operation="delete", workflow_id=workflow_id) from e
 
     payload = {"workflow_id": workflow_id, "deleted": True}

comfy_cli/error_codes.py

@@ -80,6 +80,14 @@ class ErrorCode:
         "truncate it into a corrupt/partial file. `details.limit_bytes` carries the cap.",
         "the saved workflow is unexpectedly large; inspect it directly on the server",
     ),
+    ErrorCode(
+        "workflow_unparseable",
+        "A cloud `/api/workflows` call returned a non-empty 200 body that couldn't be decoded as JSON "
+        "(non-UTF-8 bytes or a non-JSON body such as an HTML proxy/error page). Distinct from an empty "
+        "body (legitimately no data): the malformed body is surfaced as a hard error rather than a "
+        "misleading empty list / null id. `details.operation` carries the verb.",
+        "the server sent a malformed body; retry, and report it if it persists",
+    ),
     ErrorCode(
         "workflow_content_not_json",
         "`workflow get` fetched content that isn't parseable JSON (non-UTF-8 bytes or a non-JSON body such "

tests/comfy_cli/command/test_workflow_saved.py

@@ -531,3 +531,79 @@ def test_404_surfaces_workflow_not_found(self, cloud_target, monkeypatch, capsys
         env = _run(["delete", "ghost", "--where", "cloud"], capsys)
         assert env["ok"] is False
         assert env["error"]["code"] == "workflow_not_found"
+
+
+# ---------------------------------------------------------------------------
+# unparseable 200 body — must surface a loud error, not empty/success (BE-3334)
+# ---------------------------------------------------------------------------
+
+# A non-empty 200 body the client can't decode as JSON. Four shapes, all malformed:
+#   - a valid-UTF-8 non-JSON page (e.g. an HTML proxy/error page returned 200) -> JSONDecodeError;
+#   - a non-UTF-8 byte string that isn't valid UTF-8 at all -> UnicodeDecodeError;
+#   - valid JSON encoded as UTF-16, which the *contract* treats as malformed. ``json.loads``
+#     auto-detects UTF-16/32 byte input (RFC 4627), so handed raw bytes it would silently
+#     accept this; decoding as UTF-8 first surfaces it as UnicodeDecodeError. Locks that in.
+#   - a JSON integer past CPython's 4300-digit int/str conversion limit: ``json.loads`` raises
+#     a *bare* ``ValueError`` (not ``JSONDecodeError``), which the narrow catch would have let
+#     escape as a raw traceback. Locks in the broadened ``ValueError``-base catch.
+# ``JSONDecodeError`` and ``UnicodeDecodeError`` both subclass ``ValueError``; catching the base
+# maps all four. None may be reported as "no data" (empty list / null id).
+_UNPARSEABLE_BODIES = [
+    pytest.param(b"<html>502 Bad Gateway</html>", id="non-json"),
+    pytest.param(b"\xff\xfe\x00bad", id="non-utf8"),
+    pytest.param(json.dumps({"data": [], "id": "x"}).encode("utf-16"), id="utf16-json"),
+    pytest.param(b"1" + b"0" * 4400, id="bigint-valueerror"),
+]
+
+
+class TestUnparseableResponse:
+    @pytest.mark.parametrize("raw", _UNPARSEABLE_BODIES)
+    def test_list_surfaces_error_not_empty(self, cloud_target, monkeypatch, capsys, raw):
+        _patch_urlopen(monkeypatch, {"/api/workflows": (raw, 200)})
+        env = _run(["list", "--where", "cloud"], capsys)
+        # Must NOT masquerade as a successful, genuinely-empty list.
+        assert env["ok"] is False
+        assert env["error"]["code"] == "workflow_unparseable"
+        assert env["error"]["details"]["operation"] == "list"
+
+    @pytest.mark.parametrize("raw", _UNPARSEABLE_BODIES)
+    def test_save_surfaces_error_not_null_id(self, cloud_target, tmp_path, monkeypatch, capsys, raw):
+        wf_path = tmp_path / "wf.json"
+        wf_path.write_text(json.dumps({"1": {"class_type": "KSampler", "inputs": {}}}))
+        _patch_urlopen(monkeypatch, {"/api/workflows": (raw, 200)})
+        env = _run(["save", str(wf_path), "--name", "x", "--where", "cloud"], capsys)
+        # Must NOT claim success with a null workflow_id.
+        assert env["ok"] is False
+        assert env["error"]["code"] == "workflow_unparseable"
+        assert env["error"]["details"]["operation"] == "save"
+
+    @pytest.mark.parametrize("raw", _UNPARSEABLE_BODIES)
+    def test_get_surfaces_error_not_missing_content(self, cloud_target, monkeypatch, capsys, raw):
+        _patch_urlopen(monkeypatch, {"/api/workflows/wf-uuid/content": (raw, 200)})
+        env = _run(["get", "wf-uuid", "--where", "cloud"], capsys)
+        # Must NOT be reported as missing/invalid content — it's a malformed transport body.
+        assert env["ok"] is False
+        assert env["error"]["code"] == "workflow_unparseable"
+        assert env["error"]["details"]["operation"] == "get"
+
+    @pytest.mark.parametrize("raw", _UNPARSEABLE_BODIES)
+    def test_delete_surfaces_error_not_success(self, cloud_target, monkeypatch, capsys, raw):
+        _patch_urlopen(monkeypatch, {"/api/workflows/wf-uuid": (raw, 200)})
+        env = _run(["delete", "wf-uuid", "--where", "cloud"], capsys)
+        # Must NOT claim a successful delete off an unparseable body.
+        assert env["ok"] is False
+        assert env["error"]["code"] == "workflow_unparseable"
+        assert env["error"]["details"]["operation"] == "delete"
+
+
+class TestListMalformedShape:
+    """A valid-JSON but wrongly-shaped 200 body reaches ``list`` (``get``/``save`` already
+    guard with ``isinstance``). It must map to an error, not raise a raw ``AttributeError``
+    and not coerce to a masquerading empty list."""
+
+    @pytest.mark.parametrize("raw", [pytest.param(b"[1, 2, 3]", id="array"), pytest.param(b"42", id="scalar")])
+    def test_list_non_dict_body_surfaces_error(self, cloud_target, monkeypatch, capsys, raw):
+        _patch_urlopen(monkeypatch, {"/api/workflows": (raw, 200)})
+        env = _run(["list", "--where", "cloud"], capsys)
+        assert env["ok"] is False
+        assert env["error"]["code"] == "cloud_http_error"