Merge 1339cb570d into b0f9e326af

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

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 "<source>:<module_name>"
+# 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():

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.

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"}