mirror of
https://github.com/comfyanonymous/ComfyUI.git
synced 2026-05-14 19:17:32 +08:00
Categorize startup errors by source (custom_node / comfy_extra / api_node)
Expand custom-node startup error tracking to differentiate between user-installed custom_nodes, built-in comfy_extras, and partner comfy_api_nodes. Each NODE_STARTUP_ERRORS entry now carries a 'source' field and is keyed by '<source>:<module_name>' so colliding module names across the three locations don't overwrite each other. The /custom_node_startup_errors endpoint returns errors grouped by source so the frontend/Manager can render distinct sections. Also captures previously-missed failures from comfy_entrypoint() (phase='entrypoint'). Introduces nodes.record_node_startup_error() helper used by load_custom_node and main.execute_prestartup_script. Adds tests-unit/node_startup_errors_test.py (6 tests) covering field shape, source mapping for each module_parent, cross-source collisions, and default fallback. Ref: ComfyUI-Launcher#303 Amp-Thread-ID: https://ampcode.com/threads/T-019e23a1-2acc-7619-bd0e-f783d1368ef3 Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
parent
a145651cc0
commit
3a649984f2
16
main.py
16
main.py
@ -141,14 +141,14 @@ def execute_prestartup_script():
|
|||||||
except Exception as e:
|
except Exception as e:
|
||||||
import traceback
|
import traceback
|
||||||
logging.error(f"Failed to execute startup-script: {script_path} / {e}")
|
logging.error(f"Failed to execute startup-script: {script_path} / {e}")
|
||||||
from nodes import NODE_STARTUP_ERRORS, get_module_name
|
from nodes import record_node_startup_error
|
||||||
node_module_name = get_module_name(os.path.dirname(script_path))
|
record_node_startup_error(
|
||||||
NODE_STARTUP_ERRORS[node_module_name] = {
|
module_path=os.path.dirname(script_path),
|
||||||
"module_path": os.path.dirname(script_path),
|
source="custom_node",
|
||||||
"error": str(e),
|
phase="prestartup",
|
||||||
"traceback": traceback.format_exc(),
|
error=e,
|
||||||
"phase": "prestartup",
|
tb=traceback.format_exc(),
|
||||||
}
|
)
|
||||||
return False
|
return False
|
||||||
|
|
||||||
node_paths = folder_paths.get_folder_paths("custom_nodes")
|
node_paths = folder_paths.get_folder_paths("custom_nodes")
|
||||||
|
|||||||
59
nodes.py
59
nodes.py
@ -2181,10 +2181,42 @@ EXTENSION_WEB_DIRS = {}
|
|||||||
# Dictionary of successfully loaded module names and associated directories.
|
# Dictionary of successfully loaded module names and associated directories.
|
||||||
LOADED_MODULE_DIRS = {}
|
LOADED_MODULE_DIRS = {}
|
||||||
|
|
||||||
# Dictionary of custom node startup errors, keyed by module name.
|
# 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 "<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.
|
||||||
NODE_STARTUP_ERRORS: dict[str, dict] = {}
|
NODE_STARTUP_ERRORS: dict[str, dict] = {}
|
||||||
|
|
||||||
|
|
||||||
|
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)
|
||||||
|
NODE_STARTUP_ERRORS[f"{source}:{module_name}"] = {
|
||||||
|
"source": source,
|
||||||
|
"module_name": module_name,
|
||||||
|
"module_path": module_path,
|
||||||
|
"error": str(error),
|
||||||
|
"traceback": tb,
|
||||||
|
"phase": phase,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def get_module_name(module_path: str) -> str:
|
def get_module_name(module_path: str) -> str:
|
||||||
"""
|
"""
|
||||||
Returns the module name based on the given module path.
|
Returns the module name based on the given module path.
|
||||||
@ -2293,21 +2325,30 @@ async def load_custom_node(module_path: str, ignore=set(), module_parent="custom
|
|||||||
NODE_DISPLAY_NAME_MAPPINGS[schema.node_id] = schema.display_name
|
NODE_DISPLAY_NAME_MAPPINGS[schema.node_id] = schema.display_name
|
||||||
return True
|
return True
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
|
tb = traceback.format_exc()
|
||||||
logging.warning(f"Error while calling comfy_entrypoint in {module_path}: {e}")
|
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),
|
||||||
|
phase="entrypoint",
|
||||||
|
error=e,
|
||||||
|
tb=tb,
|
||||||
|
)
|
||||||
return False
|
return False
|
||||||
else:
|
else:
|
||||||
logging.warning(f"Skip {module_path} module for custom nodes due to the lack of NODE_CLASS_MAPPINGS or NODES_LIST (need one).")
|
logging.warning(f"Skip {module_path} module for custom nodes due to the lack of NODE_CLASS_MAPPINGS or NODES_LIST (need one).")
|
||||||
return False
|
return False
|
||||||
except Exception as e:
|
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}")
|
logging.warning(f"Cannot import {module_path} module for custom nodes: {e}")
|
||||||
module_name = get_module_name(module_path)
|
record_node_startup_error(
|
||||||
NODE_STARTUP_ERRORS[module_name] = {
|
module_path=module_path,
|
||||||
"module_path": module_path,
|
source=_node_source_from_parent(module_parent),
|
||||||
"error": str(e),
|
phase="import",
|
||||||
"traceback": traceback.format_exc(),
|
error=e,
|
||||||
"phase": "import",
|
tb=tb,
|
||||||
}
|
)
|
||||||
return False
|
return False
|
||||||
|
|
||||||
async def init_external_custom_nodes():
|
async def init_external_custom_nodes():
|
||||||
|
|||||||
12
server.py
12
server.py
@ -755,7 +755,17 @@ class PromptServer():
|
|||||||
|
|
||||||
@routes.get("/custom_node_startup_errors")
|
@routes.get("/custom_node_startup_errors")
|
||||||
async def get_custom_node_startup_errors(request):
|
async def get_custom_node_startup_errors(request):
|
||||||
return web.json_response(nodes.NODE_STARTUP_ERRORS)
|
# Group errors by source ("custom_node" / "comfy_extra" / "api_node")
|
||||||
|
# so the frontend/Manager can render them in distinct sections.
|
||||||
|
grouped: dict[str, dict[str, dict]] = {
|
||||||
|
"custom_node": {},
|
||||||
|
"comfy_extra": {},
|
||||||
|
"api_node": {},
|
||||||
|
}
|
||||||
|
for entry in nodes.NODE_STARTUP_ERRORS.values():
|
||||||
|
source = entry.get("source", "custom_node")
|
||||||
|
grouped.setdefault(source, {})[entry["module_name"]] = entry
|
||||||
|
return web.json_response(grouped)
|
||||||
|
|
||||||
@routes.get("/api/jobs")
|
@routes.get("/api/jobs")
|
||||||
async def get_jobs(request):
|
async def get_jobs(request):
|
||||||
|
|||||||
104
tests-unit/node_startup_errors_test.py
Normal file
104
tests-unit/node_startup_errors_test.py
Normal file
@ -0,0 +1,104 @@
|
|||||||
|
"""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.
|
||||||
|
"""
|
||||||
|
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_node",
|
||||||
|
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 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,expected_source",
|
||||||
|
[
|
||||||
|
("custom_nodes", "custom_node"),
|
||||||
|
("comfy_extras", "comfy_extra"),
|
||||||
|
("comfy_api_nodes", "api_node"),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
async def test_load_custom_node_records_source(tmp_path, module_parent, expected_source):
|
||||||
|
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"
|
||||||
|
assert key in nodes.NODE_STARTUP_ERRORS, nodes.NODE_STARTUP_ERRORS
|
||||||
|
entry = nodes.NODE_STARTUP_ERRORS[key]
|
||||||
|
assert entry["source"] == expected_source
|
||||||
|
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_node:nodes_audio" in nodes.NODE_STARTUP_ERRORS
|
||||||
|
assert "comfy_extra: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"]
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
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"
|
||||||
Loading…
Reference in New Issue
Block a user