diff --git a/main.py b/main.py index 153dc229a..139eefbae 100644 --- a/main.py +++ b/main.py @@ -144,7 +144,7 @@ def execute_prestartup_script(): from nodes import record_node_startup_error record_node_startup_error( module_path=os.path.dirname(script_path), - source="custom_node", + source="custom_nodes", phase="prestartup", error=e, tb=traceback.format_exc(), diff --git a/nodes.py b/nodes.py index 0775a78f2..f2b3be120 100644 --- a/nodes.py +++ b/nodes.py @@ -2181,24 +2181,17 @@ EXTENSION_WEB_DIRS = {} # Dictionary of successfully loaded module names and associated directories. LOADED_MODULE_DIRS = {} -# Mapping from internal module_parent values to the public "source" -# category the API exposes. Keeps the on-disk layout decoupled from -# the names the frontend/Manager switches on. -_NODE_SOURCE_BY_PARENT = { - "custom_nodes": "custom_node", - "comfy_extras": "comfy_extra", - "comfy_api_nodes": "api_node", -} - - -def _node_source_from_parent(module_parent: str) -> str: - return _NODE_SOURCE_BY_PARENT.get(module_parent, "custom_node") - - # Dictionary of custom node startup errors, keyed by ":" # so that name collisions across custom_nodes / comfy_extras / comfy_api_nodes # do not overwrite each other. Each value contains: source, module_name, # module_path, error, traceback, phase. +# +# `source` is the same string as the internal `module_parent` used at load +# time (e.g. "custom_nodes", "comfy_extras", "comfy_api_nodes"). It is +# intentionally a free-form string rather than a fixed enum so the contract +# survives node-source layouts evolving (e.g. comfy_api_nodes eventually +# moving out of core). Consumers should treat any new value as a new bucket +# rather than rejecting it. NODE_STARTUP_ERRORS: dict[str, dict] = {} @@ -2366,7 +2359,7 @@ async def load_custom_node(module_path: str, ignore=set(), module_parent="custom logging.warning(f"Error while calling comfy_entrypoint in {module_path}: {e}") record_node_startup_error( module_path=module_path, - source=_node_source_from_parent(module_parent), + source=module_parent, phase="entrypoint", error=e, tb=tb, @@ -2381,7 +2374,7 @@ async def load_custom_node(module_path: str, ignore=set(), module_parent="custom logging.warning(f"Cannot import {module_path} module for custom nodes: {e}") record_node_startup_error( module_path=module_path, - source=_node_source_from_parent(module_parent), + source=module_parent, phase="import", error=e, tb=tb, diff --git a/server.py b/server.py index 749c89f1d..46046b1b2 100644 --- a/server.py +++ b/server.py @@ -755,17 +755,20 @@ class PromptServer(): @routes.get("/node_startup_errors") async def get_node_startup_errors(request): - # Group errors by source ("custom_node" / "comfy_extra" / "api_node") - # so the frontend/Manager can render them in distinct sections. + # Group errors by source so the frontend/Manager can render them + # in distinct sections. `source` is the same string as the + # module_parent used at load time (e.g. "custom_nodes", + # "comfy_extras", "comfy_api_nodes") and is left as a free-form + # string so the contract survives node-source layouts evolving. + # The response only contains source buckets that actually had a + # failure; consumers should not assume any particular set of keys + # is always present. + # # `module_path` is stripped because the absolute on-disk path is # internal detail that the frontend has no use for. - grouped: dict[str, dict[str, dict]] = { - "custom_node": {}, - "comfy_extra": {}, - "api_node": {}, - } + grouped: dict[str, dict[str, dict]] = {} for entry in nodes.NODE_STARTUP_ERRORS.values(): - source = entry.get("source", "custom_node") + source = entry.get("source", "custom_nodes") public_entry = {k: v for k, v in entry.items() if k != "module_path"} grouped.setdefault(source, {})[entry["module_name"]] = public_entry return web.json_response(grouped) diff --git a/tests-unit/node_startup_errors_test.py b/tests-unit/node_startup_errors_test.py index 533ba78e7..5395550c3 100644 --- a/tests-unit/node_startup_errors_test.py +++ b/tests-unit/node_startup_errors_test.py @@ -7,6 +7,7 @@ Covers: - Composite keying prevents collisions between modules with the same name in different sources. - record_node_startup_error stores the expected fields. +- pyproject.toml metadata is attached when present and omitted when absent. """ import textwrap @@ -35,14 +36,14 @@ def test_record_node_startup_error_fields(tmp_path): err = ValueError("kaboom") nodes.record_node_startup_error( module_path=str(tmp_path / "my_pack"), - source="custom_node", + source="custom_nodes", phase="import", error=err, tb="traceback-text", ) - assert "custom_node:my_pack" in nodes.NODE_STARTUP_ERRORS - entry = nodes.NODE_STARTUP_ERRORS["custom_node:my_pack"] - assert entry["source"] == "custom_node" + assert "custom_nodes:my_pack" in nodes.NODE_STARTUP_ERRORS + entry = nodes.NODE_STARTUP_ERRORS["custom_nodes:my_pack"] + assert entry["source"] == "custom_nodes" assert entry["module_name"] == "my_pack" assert entry["phase"] == "import" assert entry["error"] == "kaboom" @@ -52,23 +53,20 @@ def test_record_node_startup_error_fields(tmp_path): @pytest.mark.asyncio @pytest.mark.parametrize( - "module_parent,expected_source", - [ - ("custom_nodes", "custom_node"), - ("comfy_extras", "comfy_extra"), - ("comfy_api_nodes", "api_node"), - ], + "module_parent", + ["custom_nodes", "comfy_extras", "comfy_api_nodes"], ) -async def test_load_custom_node_records_source(tmp_path, module_parent, expected_source): +async def test_load_custom_node_records_source(tmp_path, module_parent): + # `source` in the entry should be the same string as `module_parent`. module_path = _write_broken_module(tmp_path, "broken_pack") success = await nodes.load_custom_node(module_path, module_parent=module_parent) assert success is False - key = f"{expected_source}:broken_pack" + key = f"{module_parent}:broken_pack" assert key in nodes.NODE_STARTUP_ERRORS, nodes.NODE_STARTUP_ERRORS entry = nodes.NODE_STARTUP_ERRORS[key] - assert entry["source"] == expected_source + assert entry["source"] == module_parent assert entry["module_name"] == "broken_pack" assert entry["phase"] == "import" assert "boom from" in entry["error"] @@ -77,7 +75,7 @@ async def test_load_custom_node_records_source(tmp_path, module_parent, expected @pytest.mark.asyncio async def test_load_custom_node_collision_across_sources(tmp_path): - # Same module name registered as both a custom_node and a comfy_extra; + # Same module name registered as both a custom node and a comfy_extra; # composite keying should keep both entries. cn_dir = tmp_path / "cn" extras_dir = tmp_path / "extras" @@ -89,11 +87,11 @@ async def test_load_custom_node_collision_across_sources(tmp_path): assert await nodes.load_custom_node(cn_path, module_parent="custom_nodes") is False assert await nodes.load_custom_node(extras_path, module_parent="comfy_extras") is False - assert "custom_node:nodes_audio" in nodes.NODE_STARTUP_ERRORS - assert "comfy_extra:nodes_audio" in nodes.NODE_STARTUP_ERRORS + assert "custom_nodes:nodes_audio" in nodes.NODE_STARTUP_ERRORS + assert "comfy_extras:nodes_audio" in nodes.NODE_STARTUP_ERRORS assert ( - nodes.NODE_STARTUP_ERRORS["custom_node:nodes_audio"]["module_path"] - != nodes.NODE_STARTUP_ERRORS["comfy_extra:nodes_audio"]["module_path"] + nodes.NODE_STARTUP_ERRORS["custom_nodes:nodes_audio"]["module_path"] + != nodes.NODE_STARTUP_ERRORS["comfy_extras:nodes_audio"]["module_path"] ) @@ -118,7 +116,7 @@ async def test_load_custom_node_attaches_pyproject_metadata(tmp_path): success = await nodes.load_custom_node(str(pack_dir), module_parent="custom_nodes") assert success is False - entry = nodes.NODE_STARTUP_ERRORS["custom_node:MyCoolPack"] + entry = nodes.NODE_STARTUP_ERRORS["custom_nodes:MyCoolPack"] assert "pyproject" in entry, entry py = entry["pyproject"] assert py["pack_id"] == "comfyui-mycoolpack" @@ -134,12 +132,15 @@ async def test_load_custom_node_no_pyproject_skips_metadata(tmp_path): # so the entry must not contain a 'pyproject' key. module_path = _write_broken_module(tmp_path, "lonely") assert await nodes.load_custom_node(module_path, module_parent="comfy_extras") is False - entry = nodes.NODE_STARTUP_ERRORS["comfy_extra:lonely"] + entry = nodes.NODE_STARTUP_ERRORS["comfy_extras:lonely"] assert "pyproject" not in entry -def test_unknown_module_parent_defaults_to_custom_node(): - assert nodes._node_source_from_parent("custom_nodes") == "custom_node" - assert nodes._node_source_from_parent("comfy_extras") == "comfy_extra" - assert nodes._node_source_from_parent("comfy_api_nodes") == "api_node" - assert nodes._node_source_from_parent("something_else") == "custom_node" +@pytest.mark.asyncio +async def test_load_custom_node_arbitrary_module_parent_passes_through(tmp_path): + # `source` is a free-form string — an unknown module_parent (e.g. a future + # node-source bucket) should be recorded as-is, not coerced or rejected. + module_path = _write_broken_module(tmp_path, "future_pack") + assert await nodes.load_custom_node(module_path, module_parent="future_source") is False + entry = nodes.NODE_STARTUP_ERRORS["future_source:future_pack"] + assert entry["source"] == "future_source"