diff --git a/comfy_cli/command/assets_library.py b/comfy_cli/command/assets_library.py index 07ca2cfd9..a0f104048 100644 --- a/comfy_cli/command/assets_library.py +++ b/comfy_cli/command/assets_library.py @@ -123,10 +123,24 @@ def ensure_cmd( resource_id=hash, ) from e - b = body or {} + # Unlike `ls`, an EMPTY body is an error here, not an empty result: a POST + # cannot confirm the borrow without one, and the server contract + # (`AssetCreated`) always returns an object. `http_request` also collapses + # an unparseable body (an HTML error page from a proxy, say) to `None`, so + # `NoneType` covers both — otherwise they emit `{"ok": true}` with a null id + # and the caller's own hash echoed back as if the borrow had happened. + if not isinstance(body, dict): + renderer.error( + code="cloud_http_error", + message="unexpected response from /api/assets/from-hash (expected a JSON object describing the asset)", + hint="the borrow could not be confirmed; retry, and check whether a proxy is intercepting the request", + details={"operation": "ensure", "hash": hash, "got_type": type(body).__name__}, + ) + raise typer.Exit(code=1) + payload = { - "id": b.get("id"), - "hash": b.get("hash", hash), + "id": body.get("id"), + "hash": body.get("hash", hash), "created_new": status == 201, } renderer.emit(payload, command="assets library ensure", where="cloud") diff --git a/tests/comfy_cli/command/test_assets_library.py b/tests/comfy_cli/command/test_assets_library.py index d8477caa1..8ac37ae2e 100644 --- a/tests/comfy_cli/command/test_assets_library.py +++ b/tests/comfy_cli/command/test_assets_library.py @@ -89,6 +89,35 @@ def _fake(req, timeout=None): return calls +def _patch_urlopen_raw(monkeypatch: pytest.MonkeyPatch, raw: bytes, status: int = 201): + """Serve ``raw`` verbatim, unlike ``_patch_urlopen`` which JSON-encodes it. + + Needed for the bodies a real proxy/gateway returns — an HTML error page, + nothing at all — which never survive a ``json.dumps`` round trip. + """ + calls: list[dict] = [] + + class _Resp: + def __init__(self): + self.status = status + + def __enter__(self): + return self + + def __exit__(self, *a): + return False + + def read(self, n=None): + return raw + + def _fake(req, timeout=None): + calls.append({"url": req.full_url, "method": req.get_method(), "body": req.data}) + return _Resp() + + monkeypatch.setattr("urllib.request.urlopen", _fake) + return calls + + class TestEnsure: def test_404_is_asset_not_found_not_workflow_not_found(self, cloud_target, monkeypatch, capsys): _patch_urlopen(monkeypatch, _http_error(404)) @@ -105,6 +134,44 @@ def test_404_is_asset_not_found_not_workflow_not_found(self, cloud_target, monke assert err["details"]["hash"] == "comfyorg_logo.png" assert err["details"]["operation"] == "ensure" + def test_non_object_body_is_cloud_http_error(self, cloud_target, monkeypatch, capsys): + # Pre-fix: `AttributeError: 'list' object has no attribute 'get'` and no envelope at all. + _patch_urlopen(monkeypatch, [1, 2, 3]) + env = _run(["ensure", "--hash", "a" * 64, "--where", "cloud"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "cloud_http_error" + assert error_codes.is_registered(env["error"]["code"]) + assert env["error"]["details"]["got_type"] == "list" + assert env["error"]["details"]["operation"] == "ensure" + assert "created_new" not in json.dumps(env) + + def test_unparseable_body_is_cloud_http_error_not_fake_success(self, cloud_target, monkeypatch, capsys): + # Pre-fix: `{"ok": true, "data": {"id": null, "hash": , "created_new": true}}` + # — a borrow the server never confirmed, reported as done. + _patch_urlopen_raw(monkeypatch, b"502 Bad Gateway") + env = _run(["ensure", "--hash", "a" * 64, "--where", "cloud"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "cloud_http_error" + assert error_codes.is_registered(env["error"]["code"]) + assert "created_new" not in json.dumps(env) + + def test_empty_body_is_cloud_http_error(self, cloud_target, monkeypatch, capsys): + # A POST with no body cannot confirm the borrow: unlike `ls`, empty is an error here. + _patch_urlopen_raw(monkeypatch, b"", status=200) + env = _run(["ensure", "--hash", "a" * 64, "--where", "cloud"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "cloud_http_error" + assert error_codes.is_registered(env["error"]["code"]) + assert "created_new" not in json.dumps(env) + + def test_whitespace_body_is_cloud_http_error(self, cloud_target, monkeypatch, capsys): + _patch_urlopen_raw(monkeypatch, b"\n") + env = _run(["ensure", "--hash", "a" * 64, "--where", "cloud"], capsys) + assert env["ok"] is False + assert env["error"]["code"] == "cloud_http_error" + assert error_codes.is_registered(env["error"]["code"]) + assert "created_new" not in json.dumps(env) + def test_401_is_still_cloud_unauthorized(self, cloud_target, monkeypatch, capsys): _patch_urlopen(monkeypatch, _http_error(401)) env = _run(["ensure", "--hash", "a" * 64, "--where", "cloud"], capsys)