diff --git a/main.py b/main.py index ad5c11e16..942aebd96 100644 --- a/main.py +++ b/main.py @@ -28,6 +28,7 @@ import faulthandler import logging import signal import sys +import traceback from comfy_execution.progress import get_progress_state from comfy_execution.utils import get_executing_context from comfy_api import feature_flags @@ -156,7 +157,20 @@ def apply_custom_paths(): folder_paths.set_user_directory(user_dir) +# Buffer for prestartup failures. Recorded into `nodes.NODE_STARTUP_ERRORS` +# only AFTER the normal `import nodes` line below, so a failing prestartup +# script never triggers an early `import nodes` (and therefore `import torch`) +# on the error path. +_PRESTARTUP_FAILURES: list[dict] = [] + + def execute_prestartup_script(): + """Run every custom_nodes/*/prestartup_script.py once, before importing nodes. + + Failures are buffered into the module-level ``_PRESTARTUP_FAILURES`` list and + must be flushed via ``record_node_startup_error`` after ``import nodes`` has + happened at its normal bootstrap point. + """ if args.disable_all_custom_nodes and len(args.whitelist_custom_nodes) == 0: return @@ -169,6 +183,15 @@ def execute_prestartup_script(): return True except Exception as e: logging.error(f"Failed to execute startup-script: {script_path} / {e}") + # Buffer the failure - do NOT `import nodes` here, that would drag + # torch in before the intended bootstrap point. + _PRESTARTUP_FAILURES.append({ + "module_path": os.path.dirname(script_path), + "source": "custom_nodes", + "phase": "prestartup", + "error": e, + "tb": traceback.format_exc(), + }) return False node_paths = folder_paths.get_folder_paths("custom_nodes") @@ -228,6 +251,16 @@ import execution import server from protocol import BinaryEventTypes import nodes + +# Flush any prestartup failures that were buffered before `nodes` was +# importable. Doing this here (rather than from the prestartup error +# handler) keeps the bootstrap order deterministic: `nodes` (and torch) +# import at this single line whether prestartup succeeded or failed. +if _PRESTARTUP_FAILURES: + for _failure in _PRESTARTUP_FAILURES: + nodes.record_node_startup_error(**_failure) + _PRESTARTUP_FAILURES.clear() + import comfy.model_management import comfyui_version import app.logger diff --git a/nodes.py b/nodes.py index 66c08121d..af12d740c 100644 --- a/nodes.py +++ b/nodes.py @@ -2174,6 +2174,137 @@ EXTENSION_WEB_DIRS = {} # Dictionary of successfully loaded module names and associated directories. LOADED_MODULE_DIRS = {} +# 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] = {} + + +_EMPTY_LEAF_VALUES = (None, "", [], {}) + + +def _prune_empty(value): + """Recursively drop empty strings / lists / dicts / None from a nested structure. + + Used to keep the on-wire pyproject payload tight without altering the + nesting that callers see (so consumers can still parse it back through + ``PyProjectConfig`` if they want a typed object). + """ + if isinstance(value, dict): + out = {} + for k, v in value.items(): + cleaned = _prune_empty(v) + if cleaned not in _EMPTY_LEAF_VALUES: + out[k] = cleaned + return out + if isinstance(value, list): + return [ + cleaned + for cleaned in (_prune_empty(v) for v in value) + if cleaned not in _EMPTY_LEAF_VALUES + ] + return value + + +def _read_pyproject_metadata(module_path: str) -> dict | None: + """Best-effort extraction of pyproject.toml for a node module. + + Returns a dict mirroring the ``PyProjectConfig`` shape produced by + ``comfy_config.config_parser.extract_node_configuration`` (i.e. with + ``project`` and ``tool_comfy`` nesting and the same field names) when the + module directory contains a pyproject.toml. Empty / default-valued leaves + are pruned so the API payload stays compact, but the nesting is kept + intact so API consumers can parse the result back through + ``PyProjectConfig`` directly. + + Returns None when no toml is present or parsing fails for any reason — + startup-error tracking must never itself raise. + """ + if not module_path or not os.path.isdir(module_path): + return None + toml_path = os.path.join(module_path, "pyproject.toml") + if not os.path.isfile(toml_path): + return None + try: + from comfy_config import config_parser + + cfg = config_parser.extract_node_configuration(module_path) + if cfg is None: + return None + pruned = _prune_empty(cfg.model_dump()) + return pruned or None + except Exception: + return None + + +def record_node_startup_error( + *, module_path: str, source: str, phase: str, error: BaseException, tb: str +) -> None: + """Record a startup error for a node module so it can be exposed via the API.""" + module_name = get_module_name(module_path) + entry = { + "source": source, + "module_name": module_name, + "module_path": module_path, + "error": str(error), + "traceback": tb, + "phase": phase, + } + pyproject = _read_pyproject_metadata(module_path) + if pyproject: + entry["pyproject"] = pyproject + NODE_STARTUP_ERRORS[f"{source}:{module_name}"] = entry + + +def filter_node_startup_errors( + *, + source: str | None = None, + module_name: str | None = None, + pack_id: str | None = None, +) -> dict[str, dict[str, dict]]: + """Return `NODE_STARTUP_ERRORS` reshaped for the public HTTP endpoint. + + Entries are grouped by their ``source`` bucket (the same string as the + internal ``module_parent`` used at load time). The on-disk + ``module_path`` is stripped from each entry — it's an internal detail + useful only for server-side logging and would leak absolute filesystem + layout otherwise. + + Optional filters narrow the response and combine with AND: + + * ``source`` — only entries from this source bucket. + * ``module_name`` — only entries whose module name matches exactly. + * ``pack_id`` — only entries whose ``pyproject.project.name`` + matches exactly. Entries without a parsed + pyproject.toml can never match this filter. + + A non-matching filter returns an empty dict, not an error — absence of + a failure is a valid answer for this query. + """ + grouped: dict[str, dict[str, dict]] = {} + for entry in NODE_STARTUP_ERRORS.values(): + entry_source = entry.get("source", "custom_nodes") + if source is not None and entry_source != source: + continue + if module_name is not None and entry.get("module_name") != module_name: + continue + if pack_id is not None: + pyproject = entry.get("pyproject") or {} + project = pyproject.get("project") or {} + if project.get("name") != pack_id: + continue + public_entry = {k: v for k, v in entry.items() if k != "module_path"} + grouped.setdefault(entry_source, {})[entry["module_name"]] = public_entry + return grouped + def get_module_name(module_path: str) -> str: """ @@ -2283,14 +2414,30 @@ async def load_custom_node(module_path: str, ignore=set(), module_parent="custom NODE_DISPLAY_NAME_MAPPINGS[schema.node_id] = schema.display_name return True except Exception as e: + tb = traceback.format_exc() logging.warning(f"Error while calling comfy_entrypoint in {module_path}: {e}") + record_node_startup_error( + module_path=module_path, + source=module_parent, + phase="entrypoint", + error=e, + tb=tb, + ) return False else: logging.warning(f"Skip {module_path} module for custom nodes due to the lack of NODE_CLASS_MAPPINGS or comfy_entrypoint (need one).") return False except Exception as e: - logging.warning(traceback.format_exc()) + tb = traceback.format_exc() + logging.warning(tb) logging.warning(f"Cannot import {module_path} module for custom nodes: {e}") + record_node_startup_error( + module_path=module_path, + source=module_parent, + phase="import", + error=e, + tb=tb, + ) return False async def init_external_custom_nodes(): diff --git a/server.py b/server.py index 361850f38..6c33f8057 100644 --- a/server.py +++ b/server.py @@ -784,6 +784,46 @@ class PromptServer(): out[node_class] = node_info(node_class) return web.json_response(out) + @routes.get("/node_startup_errors") + async def get_node_startup_errors(request): + """Return startup errors recorded during node loading, grouped by source. + + 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. + + Optional query parameters narrow the response: + + * ``source`` — only entries from this source bucket. + * ``module_name`` — only entries whose module name matches exactly. + (Folder name for directory-style packs, file + stem for single-file modules.) + * ``pack_id`` — only entries whose ``pyproject.project.name`` + matches exactly. Entries without a parsed + pyproject.toml are skipped under this filter. + + Filters are combined with AND. Filtering an empty / non-matching + result still returns ``{}`` with HTTP 200 rather than 404 — absence + of an error is a valid answer for this endpoint. + """ + # Coalesce empty-string query values to None so `?source=` (param + # present but blank) is treated the same as the param being absent + # — rather than filtering for entries whose source is literally "". + grouped = nodes.filter_node_startup_errors( + source=request.query.get("source") or None, + module_name=request.query.get("module_name") or None, + pack_id=request.query.get("pack_id") or None, + ) + return web.json_response(grouped) + @routes.get("/api/jobs") async def get_jobs(request): """List all jobs with filtering, sorting, and pagination. diff --git a/tests-unit/node_startup_errors_test.py b/tests-unit/node_startup_errors_test.py new file mode 100644 index 000000000..0afe01fc4 --- /dev/null +++ b/tests-unit/node_startup_errors_test.py @@ -0,0 +1,258 @@ +"""Tests for the custom node startup error tracking introduced for +Comfy-Org/ComfyUI-Launcher#303. + +Covers: +- load_custom_node populates NODE_STARTUP_ERRORS with the correct source + for each module_parent (custom_nodes / comfy_extras / comfy_api_nodes). +- 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 + +import pytest + +import nodes + + +@pytest.fixture(autouse=True) +def _clear_startup_errors(): + nodes.NODE_STARTUP_ERRORS.clear() + yield + nodes.NODE_STARTUP_ERRORS.clear() + + +def _write_broken_module(tmp_path, name: str) -> str: + path = tmp_path / f"{name}.py" + path.write_text(textwrap.dedent("""\ + # Deliberately broken module to exercise startup-error tracking. + raise RuntimeError("boom from " + __name__) + """)) + return str(path) + + +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_nodes", + phase="import", + error=err, + tb="traceback-text", + ) + 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" + assert entry["traceback"] == "traceback-text" + assert entry["module_path"].endswith("my_pack") + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "module_parent", + ["custom_nodes", "comfy_extras", "comfy_api_nodes"], +) +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"{module_parent}:broken_pack" + assert key in nodes.NODE_STARTUP_ERRORS, nodes.NODE_STARTUP_ERRORS + entry = nodes.NODE_STARTUP_ERRORS[key] + assert entry["source"] == module_parent + assert entry["module_name"] == "broken_pack" + assert entry["phase"] == "import" + assert "boom from" in entry["error"] + assert "RuntimeError" in entry["traceback"] + + +@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; + # composite keying should keep both entries. + cn_dir = tmp_path / "cn" + extras_dir = tmp_path / "extras" + cn_dir.mkdir() + extras_dir.mkdir() + cn_path = _write_broken_module(cn_dir, "nodes_audio") + extras_path = _write_broken_module(extras_dir, "nodes_audio") + + 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_nodes:nodes_audio" in nodes.NODE_STARTUP_ERRORS + assert "comfy_extras:nodes_audio" in nodes.NODE_STARTUP_ERRORS + assert ( + nodes.NODE_STARTUP_ERRORS["custom_nodes:nodes_audio"]["module_path"] + != nodes.NODE_STARTUP_ERRORS["comfy_extras:nodes_audio"]["module_path"] + ) + + +@pytest.mark.asyncio +async def test_load_custom_node_attaches_pyproject_metadata(tmp_path): + pack_dir = tmp_path / "MyCoolPack" + pack_dir.mkdir() + (pack_dir / "__init__.py").write_text("raise RuntimeError('boom')\n") + (pack_dir / "pyproject.toml").write_text(textwrap.dedent("""\ + [project] + name = "comfyui-mycoolpack" + version = "1.2.3" + + [project.urls] + Repository = "https://github.com/example/comfyui-mycoolpack" + + [tool.comfy] + PublisherId = "example" + DisplayName = "My Cool Pack" + """)) + + success = await nodes.load_custom_node(str(pack_dir), module_parent="custom_nodes") + assert success is False + + entry = nodes.NODE_STARTUP_ERRORS["custom_nodes:MyCoolPack"] + assert "pyproject" in entry, entry + py = entry["pyproject"] + + # Shape must mirror PyProjectConfig 1:1 so consumers can parse it back + # through the same pydantic model used by comfy_config.config_parser. + project = py["project"] + assert project["name"] == "comfyui-mycoolpack" + assert project["version"] == "1.2.3" + assert project["urls"]["repository"] == "https://github.com/example/comfyui-mycoolpack" + + tool_comfy = py["tool_comfy"] + assert tool_comfy["publisher_id"] == "example" + assert tool_comfy["display_name"] == "My Cool Pack" + + +def test_prune_empty_drops_empty_leaves_only(): + src = { + "keep_str": "x", + "drop_empty_str": "", + "drop_none": None, + "drop_empty_list": [], + "drop_empty_dict": {}, + "keep_zero": 0, + "keep_false": False, + "nested": { + "drop_me": "", + "keep_me": "y", + "deeper": {"only_empties": ""}, + }, + "list_of_dicts": [{"a": ""}, {"a": "z"}], + } + result = nodes._prune_empty(src) + assert result == { + "keep_str": "x", + "keep_zero": 0, + "keep_false": False, + "nested": {"keep_me": "y"}, + "list_of_dicts": [{"a": "z"}], + } + + +@pytest.mark.asyncio +async def test_load_custom_node_no_pyproject_skips_metadata(tmp_path): + # Single-file extras-style module: no pyproject.toml exists alongside it, + # 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_extras:lonely"] + assert "pyproject" not in entry + + +@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" + + +# --------------------------------------------------------------------------- +# Tests for the public reshape/filter helper (nodes.filter_node_startup_errors). +# The HTTP route is a thin wrapper around this helper, so unit-testing it +# directly avoids spinning up an aiohttp app while still covering every +# query-param branch. +# --------------------------------------------------------------------------- + + +def _seed(*, source, module_name, pack_id=None, module_path="/abs/path"): + """Insert a synthetic entry directly into NODE_STARTUP_ERRORS.""" + entry = { + "source": source, + "module_name": module_name, + "module_path": module_path, + "error": "boom", + "traceback": "tb", + "phase": "import", + } + if pack_id is not None: + entry["pyproject"] = {"project": {"name": pack_id}} + nodes.NODE_STARTUP_ERRORS[f"{source}:{module_name}"] = entry + + +def test_filter_node_startup_errors_strips_module_path_and_groups_by_source(): + _seed(source="custom_nodes", module_name="A", module_path="/x/A") + _seed(source="comfy_extras", module_name="B", module_path="/x/B") + grouped = nodes.filter_node_startup_errors() + assert set(grouped) == {"custom_nodes", "comfy_extras"} + assert "module_path" not in grouped["custom_nodes"]["A"] + assert "module_path" not in grouped["comfy_extras"]["B"] + + +def test_filter_node_startup_errors_source_filter(): + _seed(source="custom_nodes", module_name="A") + _seed(source="comfy_extras", module_name="B") + grouped = nodes.filter_node_startup_errors(source="comfy_extras") + assert set(grouped) == {"comfy_extras"} + assert set(grouped["comfy_extras"]) == {"B"} + # Non-matching source filter returns an empty dict, not an error. + assert nodes.filter_node_startup_errors(source="nope") == {} + # An explicit empty-string filter is treated as a real value (matches + # entries whose source is literally ""), NOT silently as "no filter". + # The HTTP route layer is responsible for coalescing `?source=` to None + # before calling this helper; this assertion locks that contract in. + assert nodes.filter_node_startup_errors(source="") == {} + + +def test_filter_node_startup_errors_module_name_filter(): + _seed(source="custom_nodes", module_name="A") + _seed(source="comfy_extras", module_name="A") # same name, different source + _seed(source="custom_nodes", module_name="C") + grouped = nodes.filter_node_startup_errors(module_name="A") + # Both A entries (from different sources) survive the filter and stay in + # their respective source buckets. + assert set(grouped) == {"custom_nodes", "comfy_extras"} + assert set(grouped["custom_nodes"]) == {"A"} + assert set(grouped["comfy_extras"]) == {"A"} + + +def test_filter_node_startup_errors_pack_id_filter_matches_only_pyproject_entries(): + _seed(source="custom_nodes", module_name="A", pack_id="comfyui-foo") + _seed(source="custom_nodes", module_name="B", pack_id="comfyui-bar") + _seed(source="comfy_extras", module_name="C") # no pyproject at all + grouped = nodes.filter_node_startup_errors(pack_id="comfyui-foo") + assert set(grouped) == {"custom_nodes"} + assert set(grouped["custom_nodes"]) == {"A"} + # An entry without a parsed pyproject can never match a pack_id filter. + assert nodes.filter_node_startup_errors(pack_id="anything-else") == {} + + +def test_filter_node_startup_errors_filters_combine_with_and(): + _seed(source="custom_nodes", module_name="A", pack_id="comfyui-foo") + _seed(source="comfy_extras", module_name="A", pack_id="comfyui-foo") + grouped = nodes.filter_node_startup_errors( + source="comfy_extras", pack_id="comfyui-foo" + ) + assert set(grouped) == {"comfy_extras"} + assert set(grouped["comfy_extras"]) == {"A"}