diff --git a/comfy_cli/command/custom_nodes/command.py b/comfy_cli/command/custom_nodes/command.py index d5fe48a5..b095a766 100644 --- a/comfy_cli/command/custom_nodes/command.py +++ b/comfy_cli/command/custom_nodes/command.py @@ -1219,11 +1219,6 @@ def registry_install( try: # Call the API to install the node node_version = registry_api.install_node(node_id, version) - if not node_version.download_url: - logging.error("Download URL not provided from the registry.") - ui.display_error_message(f"Failed to download the custom node {node_id}.") - return - except RegistryAPIError as e: logging.error(f"Encountered an error while installing the node. error: {str(e)}") get_renderer().error( @@ -1234,8 +1229,23 @@ def registry_install( raise typer.Exit(code=1) from e except Exception as e: logging.error(f"Encountered an error while installing the node. error: {str(e)}") - ui.display_error_message(f"Failed to download the custom node {node_id}.") - return + get_renderer().error( + code="node_install_failed", + message=f"Failed to download the custom node {node_id}.", + details={"node_id": node_id}, + ) + raise typer.Exit(code=1) from e + + # Checked outside the try: typer.Exit subclasses Exception, so raising it + # above would be swallowed by the broad handler and emit a second envelope. + if not node_version.download_url: + logging.error("Download URL not provided from the registry.") + get_renderer().error( + code="node_install_failed", + message=f"Failed to download the custom node {node_id}.", + details={"node_id": node_id}, + ) + raise typer.Exit(code=1) # Download the node archive custom_nodes_path = pathlib.Path(workspace_manager.workspace_path) / "custom_nodes" diff --git a/tests/comfy_cli/command/nodes/test_node_install.py b/tests/comfy_cli/command/nodes/test_node_install.py index 2acfb295..e2e7cf1c 100644 --- a/tests/comfy_cli/command/nodes/test_node_install.py +++ b/tests/comfy_cli/command/nodes/test_node_install.py @@ -2,6 +2,7 @@ import subprocess from unittest.mock import MagicMock, patch +import requests from typer.testing import CliRunner from comfy_cli.command.custom_nodes.command import app @@ -410,3 +411,53 @@ def test_api_error_surfaced_with_code_and_exit_1(self, tmp_path): _, kwargs = mock_get_renderer.return_value.error.call_args assert kwargs["code"] == "node_install_failed" assert kwargs["details"] == {"node_id": "test-node", "status": 404, "body": "Not Found"} + + +class TestRegistryInstallNonApiFailure: + """Failures that aren't RegistryAPIError — a connection error, a DNS failure, + a timeout, a JSON decode error, or a registry response carrying no download + URL — must also emit the node_install_failed envelope and exit non-zero. + Exiting 0 here reports a network outage to automation / CI as success.""" + + def _invoke(self, tmp_path, *, install_node_side_effect=None, install_node_return=None): + with ( + patch("comfy_cli.command.custom_nodes.command.registry_api") as mock_api, + patch("comfy_cli.command.custom_nodes.command.workspace_manager") as mock_ws, + patch("comfy_cli.command.custom_nodes.command.get_renderer") as mock_get_renderer, + patch("comfy_cli.command.custom_nodes.command.download_file") as mock_dl, + ): + mock_ws.workspace_path = str(tmp_path) + if install_node_side_effect is not None: + mock_api.install_node.side_effect = install_node_side_effect + else: + mock_api.install_node.return_value = install_node_return + + result = runner.invoke(app, ["registry-install", "test-node"]) + return result, mock_get_renderer, mock_dl + + def test_connection_error_exits_1_with_envelope(self, tmp_path): + result, mock_get_renderer, mock_dl = self._invoke( + tmp_path, install_node_side_effect=requests.ConnectionError("Name or service not known") + ) + + # Must exit non-zero so automation / CI can detect the failure. + assert result.exit_code == 1 + assert "Traceback" not in result.output + mock_dl.assert_not_called() + mock_get_renderer.return_value.error.assert_called_once() + _, kwargs = mock_get_renderer.return_value.error.call_args + assert kwargs["code"] == "node_install_failed" + assert kwargs["details"] == {"node_id": "test-node"} + + def test_missing_download_url_exits_1_with_envelope(self, tmp_path): + result, mock_get_renderer, mock_dl = self._invoke( + tmp_path, install_node_return=MagicMock(download_url="", version="1.0.0") + ) + + assert result.exit_code == 1 + assert "Traceback" not in result.output + mock_dl.assert_not_called() + mock_get_renderer.return_value.error.assert_called_once() + _, kwargs = mock_get_renderer.return_value.error.call_args + assert kwargs["code"] == "node_install_failed" + assert kwargs["details"] == {"node_id": "test-node"}