mirror of
https://github.com/comfyanonymous/ComfyUI.git
synced 2026-07-03 13:19:23 +08:00
Make node-ordering heuristics defensive instead of blaming available[0]
Addresses review feedback: the scheduler error path blamed available[0] even when picking failed while inspecting a later ready node, misreporting the node to the frontend. Instead of threading the node id through the exception, make is_output and is_async fully defensive. They are pure ordering heuristics, so a malformed node (a FUNCTION typo, or schema-derived attributes that raise) just means "not prioritized"; the node then runs through normal execution, where the error is reported against the correct node. The stage_node_execution try/except remains as a backstop only. Add a test for a node whose attribute access raises during the heuristics.
This commit is contained in:
parent
91f3c0c4d9
commit
ffdc23c6dd
@ -267,10 +267,10 @@ class ExecutionList(TopologicalSort):
|
|||||||
try:
|
try:
|
||||||
self.staged_node_id = self.ux_friendly_pick_node(available)
|
self.staged_node_id = self.ux_friendly_pick_node(available)
|
||||||
except Exception as ex:
|
except Exception as ex:
|
||||||
# Picking a node is a scheduling heuristic that inspects node
|
# Backstop: the ordering heuristics in ux_friendly_pick_node are
|
||||||
# definitions; a malformed custom node must not crash the prompt
|
# defensive, but should anything else there fail, surface it as an
|
||||||
# worker thread silently. Blame an available node and surface the
|
# execution error instead of letting it kill the prompt worker
|
||||||
# error to the frontend like any other execution error.
|
# thread. Blame an available node (best effort).
|
||||||
blamed_node = self.dynprompt.get_display_node_id(available[0])
|
blamed_node = self.dynprompt.get_display_node_id(available[0])
|
||||||
exception_type = type(ex).__qualname__
|
exception_type = type(ex).__qualname__
|
||||||
if type(ex).__module__ != "builtins":
|
if type(ex).__module__ != "builtins":
|
||||||
@ -290,29 +290,28 @@ class ExecutionList(TopologicalSort):
|
|||||||
# Technically this has no effect on the overall length of execution, but it feels better as a user
|
# Technically this has no effect on the overall length of execution, but it feels better as a user
|
||||||
# for a PreviewImage to display a result as soon as it can
|
# for a PreviewImage to display a result as soon as it can
|
||||||
# Some other heuristics could probably be used here to improve the UX further.
|
# Some other heuristics could probably be used here to improve the UX further.
|
||||||
|
# These node-ordering heuristics only affect *order*, never correctness.
|
||||||
|
# A malformed node (e.g. a FUNCTION typo, or a node whose schema-derived
|
||||||
|
# attributes raise) must not crash scheduling: failing a heuristic just
|
||||||
|
# means "not prioritized". The node then proceeds to normal execution,
|
||||||
|
# where the real error is raised and reported against the correct node.
|
||||||
def is_output(node_id):
|
def is_output(node_id):
|
||||||
class_type = self.dynprompt.get_node(node_id)["class_type"]
|
class_type = self.dynprompt.get_node(node_id)["class_type"]
|
||||||
class_def = nodes.NODE_CLASS_MAPPINGS[class_type]
|
class_def = nodes.NODE_CLASS_MAPPINGS[class_type]
|
||||||
if hasattr(class_def, 'OUTPUT_NODE') and class_def.OUTPUT_NODE == True:
|
try:
|
||||||
return True
|
return hasattr(class_def, 'OUTPUT_NODE') and class_def.OUTPUT_NODE == True
|
||||||
return False
|
except Exception:
|
||||||
|
return False
|
||||||
|
|
||||||
# If an available node is async, do that first.
|
# If an available node is async, do that first.
|
||||||
# This will execute the asynchronous function earlier, reducing the overall time.
|
# This will execute the asynchronous function earlier, reducing the overall time.
|
||||||
def is_async(node_id):
|
def is_async(node_id):
|
||||||
class_type = self.dynprompt.get_node(node_id)["class_type"]
|
class_type = self.dynprompt.get_node(node_id)["class_type"]
|
||||||
class_def = nodes.NODE_CLASS_MAPPINGS[class_type]
|
class_def = nodes.NODE_CLASS_MAPPINGS[class_type]
|
||||||
# A malformed node (e.g. FUNCTION pointing at a method that does not
|
try:
|
||||||
# exist because of a typo) must not crash scheduling here. Treat it as
|
return inspect.iscoroutinefunction(getattr(class_def, class_def.FUNCTION))
|
||||||
# non-async so it proceeds to normal execution, where the missing
|
except Exception:
|
||||||
# method raises an error that is caught and reported to the frontend.
|
|
||||||
function_name = getattr(class_def, "FUNCTION", None)
|
|
||||||
if function_name is None:
|
|
||||||
return False
|
return False
|
||||||
func = getattr(class_def, function_name, None)
|
|
||||||
if func is None:
|
|
||||||
return False
|
|
||||||
return inspect.iscoroutinefunction(func)
|
|
||||||
|
|
||||||
for node_id in node_list:
|
for node_id in node_list:
|
||||||
if is_output(node_id) or is_async(node_id):
|
if is_output(node_id) or is_async(node_id):
|
||||||
|
|||||||
@ -26,6 +26,26 @@ class _MalformedV1Node:
|
|||||||
return (None,)
|
return (None,)
|
||||||
|
|
||||||
|
|
||||||
|
class _RaisingDescriptor:
|
||||||
|
def __get__(self, obj, owner):
|
||||||
|
raise RuntimeError("schema error")
|
||||||
|
|
||||||
|
|
||||||
|
class _SchemaRaisesNode:
|
||||||
|
"""A node whose schema-derived attribute access raises, as a broken V3 node would."""
|
||||||
|
@classmethod
|
||||||
|
def INPUT_TYPES(cls):
|
||||||
|
return {"required": {}}
|
||||||
|
|
||||||
|
RETURN_TYPES = ("IMAGE",)
|
||||||
|
FUNCTION = "run"
|
||||||
|
OUTPUT_NODE = _RaisingDescriptor()
|
||||||
|
CATEGORY = "Test"
|
||||||
|
|
||||||
|
def run(self):
|
||||||
|
return (None,)
|
||||||
|
|
||||||
|
|
||||||
class _FakeOutputCache:
|
class _FakeOutputCache:
|
||||||
def all_node_ids(self):
|
def all_node_ids(self):
|
||||||
return set()
|
return set()
|
||||||
@ -51,6 +71,15 @@ def test_malformed_function_does_not_crash_scheduler():
|
|||||||
assert node_id == "1"
|
assert node_id == "1"
|
||||||
|
|
||||||
|
|
||||||
|
def test_schema_attribute_error_does_not_crash_scheduler():
|
||||||
|
"""A node whose attribute access raises during heuristics still schedules."""
|
||||||
|
execution_list = _make_execution_list("SchemaRaisesNode", _SchemaRaisesNode)
|
||||||
|
node_id, error, ex = asyncio.run(execution_list.stage_node_execution())
|
||||||
|
assert ex is None
|
||||||
|
assert error is None
|
||||||
|
assert node_id == "1"
|
||||||
|
|
||||||
|
|
||||||
def test_pick_node_failure_is_reported_not_raised():
|
def test_pick_node_failure_is_reported_not_raised():
|
||||||
"""An unexpected scheduling error is returned as an error, not raised."""
|
"""An unexpected scheduling error is returned as an error, not raised."""
|
||||||
execution_list = _make_execution_list("MalformedV1Node", _MalformedV1Node)
|
execution_list = _make_execution_list("MalformedV1Node", _MalformedV1Node)
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user