mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-20 06:20:11 +00:00
refactor(errors): extract execution utilities and fix nested subgraph error detection
Add nodeExecutionUtil.ts with getAncestorExecutionIds, getParentExecutionIds,
and buildSubgraphExecutionPaths, consolidating duplicated prefix-generation logic
from executionErrorStore.ts and app.ts.
Refactor executionErrorStore.ts:
- Extract long lastNodeErrors watcher into module-level helpers:
clearAllNodeErrorFlags, markNodeSlotErrors, applyNodeError
- Add missingAncestorExecutionIds, activeMissingNodeGraphIds,
isContainerWithMissingNode following the same pattern as
errorAncestorExecutionIds / isContainerWithInternalError
- Update LGraphNode.vue hasAnyError to include isContainerWithMissingNode,
so subgraph container nodes highlight when a nested node is missing
Fix buildSubgraphExecutionPaths to return Map<string, string[]> instead of
Map<string, string>, resolving a bug where only the first container of a
given subgraph definition was tracked; all instances are now processed.
Address CodeRabbit review items:
- Fix race condition in useErrorGroups.ts: mark all pending types RESOLVING
synchronously before the first await to prevent duplicate resolutions
- Fix NaN sort for composite execution IDs (e.g. '10:5') by replacing
Number() subtraction with localeCompare({ numeric: true })
- Localize hardcoded error strings in usePackInstall.ts and app.ts
(manager.packInstall.nodeIdRequired, g.inSubgraph)
This commit is contained in:
@@ -21,7 +21,12 @@ import { isLGraphNode } from '@/utils/litegraphUtil'
|
||||
import { isGroupNode } from '@/utils/executableGroupNodeDto'
|
||||
import { st } from '@/i18n'
|
||||
import type { MissingNodeType } from '@/types/comfy'
|
||||
import type { ErrorCardData, ErrorGroup, ErrorGroupType, ErrorItem } from './types'
|
||||
import type {
|
||||
ErrorCardData,
|
||||
ErrorGroup,
|
||||
ErrorGroupType,
|
||||
ErrorItem
|
||||
} from './types'
|
||||
import type { NodeExecutionId } from '@/types/nodeIdentification'
|
||||
import { isNodeExecutionId } from '@/types/nodeIdentification'
|
||||
|
||||
@@ -389,16 +394,21 @@ export function useErrorGroups(
|
||||
watch(
|
||||
pendingTypes,
|
||||
async (pending) => {
|
||||
for (const nodeType of pending) {
|
||||
const typeName = nodeType.type
|
||||
if (asyncResolvedIds.value.has(typeName)) continue
|
||||
const toResolve = pending.filter(
|
||||
(n) => !asyncResolvedIds.value.has(n.type)
|
||||
)
|
||||
if (!toResolve.length) return
|
||||
|
||||
const updated = new Map(asyncResolvedIds.value)
|
||||
for (const nodeType of toResolve) {
|
||||
updated.set(nodeType.type, RESOLVING)
|
||||
}
|
||||
asyncResolvedIds.value = updated
|
||||
|
||||
for (const nodeType of toResolve) {
|
||||
const pack = await inferPackFromNodeName.call(nodeType.type)
|
||||
asyncResolvedIds.value = new Map(asyncResolvedIds.value).set(
|
||||
typeName,
|
||||
RESOLVING
|
||||
)
|
||||
const pack = await inferPackFromNodeName.call(typeName)
|
||||
asyncResolvedIds.value = new Map(asyncResolvedIds.value).set(
|
||||
typeName,
|
||||
nodeType.type,
|
||||
pack?.id ?? null
|
||||
)
|
||||
}
|
||||
@@ -458,9 +468,9 @@ export function useErrorGroups(
|
||||
const typeB = typeof b === 'string' ? b : b.type
|
||||
const typeCmp = typeA.localeCompare(typeB)
|
||||
if (typeCmp !== 0) return typeCmp
|
||||
const idA = typeof a === 'string' ? 0 : Number(a.nodeId ?? 0)
|
||||
const idB = typeof b === 'string' ? 0 : Number(b.nodeId ?? 0)
|
||||
return idA - idB
|
||||
const idA = typeof a === 'string' ? '' : String(a.nodeId ?? '')
|
||||
const idB = typeof b === 'string' ? '' : String(b.nodeId ?? '')
|
||||
return idA.localeCompare(idB, undefined, { numeric: true })
|
||||
}),
|
||||
isResolving
|
||||
}))
|
||||
|
||||
@@ -71,6 +71,7 @@
|
||||
"error": "Error",
|
||||
"enter": "Enter",
|
||||
"enterSubgraph": "Enter Subgraph",
|
||||
"inSubgraph": "in subgraph '{name}'",
|
||||
"resizeFromBottomRight": "Resize from bottom-right corner",
|
||||
"resizeFromTopRight": "Resize from top-right corner",
|
||||
"resizeFromBottomLeft": "Resize from bottom-left corner",
|
||||
@@ -449,6 +450,9 @@
|
||||
"import_failed": "Import Failed"
|
||||
},
|
||||
"warningTooltip": "This package may have compatibility issues with your current environment"
|
||||
},
|
||||
"packInstall": {
|
||||
"nodeIdRequired": "Node ID is required for installation"
|
||||
}
|
||||
},
|
||||
"importFailed": {
|
||||
|
||||
@@ -344,7 +344,8 @@ const hasAnyError = computed((): boolean => {
|
||||
error ||
|
||||
executionErrorStore.getNodeErrors(nodeLocatorId.value) ||
|
||||
(lgraphNode.value &&
|
||||
executionErrorStore.isContainerWithInternalError(lgraphNode.value))
|
||||
(executionErrorStore.isContainerWithInternalError(lgraphNode.value) ||
|
||||
executionErrorStore.isContainerWithMissingNode(lgraphNode.value)))
|
||||
)
|
||||
})
|
||||
|
||||
|
||||
@@ -34,6 +34,7 @@ import {
|
||||
type NodeId,
|
||||
isSubgraphDefinition
|
||||
} from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import { buildSubgraphExecutionPaths } from '@/utils/nodeExecutionUtil'
|
||||
import type {
|
||||
ExecutionErrorWsMessage,
|
||||
NodeError,
|
||||
@@ -1188,7 +1189,9 @@ export class ComfyApp {
|
||||
type: n.type,
|
||||
nodeId: executionId,
|
||||
cnrId,
|
||||
...(displayName && { hint: `in subgraph '${displayName}'` }),
|
||||
...(displayName && {
|
||||
hint: t('g.inSubgraph', { name: displayName })
|
||||
}),
|
||||
isReplaceable: replacement !== null,
|
||||
replacement: replacement ?? undefined
|
||||
})
|
||||
@@ -1207,31 +1210,25 @@ export class ComfyApp {
|
||||
// Process nodes at the top level
|
||||
collectMissingNodesAndModels(graphData.nodes)
|
||||
|
||||
// Build map: subgraph definition UUID → container node LiteGraph ID
|
||||
// A SubgraphNode in graphData.nodes has type === subgraph definition UUID.
|
||||
const subgraphDefIds = new Set(
|
||||
graphData.definitions?.subgraphs
|
||||
?.filter(isSubgraphDefinition)
|
||||
.map((s) => s.id) ?? []
|
||||
// Build map: subgraph definition UUID → full execution path prefix.
|
||||
// Handles arbitrary nesting depth (e.g. root node 11 → "11", node 14 in sg 11 → "11:14").
|
||||
const subgraphContainerIdMap = buildSubgraphExecutionPaths(
|
||||
graphData.nodes,
|
||||
graphData.definitions?.subgraphs ?? []
|
||||
)
|
||||
const subgraphContainerIdMap = new Map<string, string>()
|
||||
for (const node of graphData.nodes ?? []) {
|
||||
if (typeof node.type === 'string' && subgraphDefIds.has(node.type)) {
|
||||
subgraphContainerIdMap.set(node.type, String(node.id))
|
||||
}
|
||||
}
|
||||
|
||||
// Process nodes in subgraphs
|
||||
if (graphData.definitions?.subgraphs) {
|
||||
for (const subgraph of graphData.definitions.subgraphs) {
|
||||
if (isSubgraphDefinition(subgraph)) {
|
||||
// Use the container node's LiteGraph ID as path prefix so the
|
||||
// resulting nodeId is a valid execution ID: "containerNodeId:localNodeId"
|
||||
collectMissingNodesAndModels(
|
||||
subgraph.nodes,
|
||||
subgraphContainerIdMap.get(subgraph.id) ?? '',
|
||||
subgraph.name || subgraph.id
|
||||
)
|
||||
const paths = subgraphContainerIdMap.get(subgraph.id) ?? []
|
||||
for (const pathPrefix of paths) {
|
||||
collectMissingNodesAndModels(
|
||||
subgraph.nodes,
|
||||
pathPrefix,
|
||||
subgraph.name || subgraph.id
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -12,15 +12,18 @@ import type {
|
||||
PromptError
|
||||
} from '@/schemas/apiSchema'
|
||||
import type { NodeId } from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import type { LGraph, LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import type { NodeExecutionId, NodeLocatorId } from '@/types/nodeIdentification'
|
||||
import {
|
||||
executionIdToNodeLocatorId,
|
||||
forEachNode,
|
||||
getNodeByExecutionId,
|
||||
getRootParentNode,
|
||||
getExecutionIdByNode
|
||||
} from '@/utils/graphTraversalUtil'
|
||||
import {
|
||||
getAncestorExecutionIds,
|
||||
getParentExecutionIds
|
||||
} from '@/utils/nodeExecutionUtil'
|
||||
import type { MissingNodeType } from '@/types/comfy'
|
||||
|
||||
interface MissingNodesError {
|
||||
@@ -28,13 +31,45 @@ interface MissingNodesError {
|
||||
nodeTypes: MissingNodeType[]
|
||||
}
|
||||
|
||||
/**
|
||||
* Store dedicated to execution error state management.
|
||||
*
|
||||
* Extracted from executionStore to separate error-related concerns
|
||||
* (state, computed properties, graph flag propagation, overlay UI)
|
||||
* from execution flow management (progress, queuing, events).
|
||||
*/
|
||||
function clearAllNodeErrorFlags(rootGraph: LGraph): void {
|
||||
forEachNode(rootGraph, (node) => {
|
||||
node.has_errors = false
|
||||
if (node.inputs) {
|
||||
for (const slot of node.inputs) {
|
||||
slot.hasErrors = false
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
function markNodeSlotErrors(node: LGraphNode, nodeError: NodeError): void {
|
||||
if (!node.inputs) return
|
||||
for (const error of nodeError.errors) {
|
||||
const slotName = error.extra_info?.input_name
|
||||
if (!slotName) continue
|
||||
const slot = node.inputs.find((s) => s.name === slotName)
|
||||
if (slot) slot.hasErrors = true
|
||||
}
|
||||
}
|
||||
|
||||
function applyNodeError(
|
||||
rootGraph: LGraph,
|
||||
executionId: string,
|
||||
nodeError: NodeError
|
||||
): void {
|
||||
const node = getNodeByExecutionId(rootGraph, executionId)
|
||||
if (!node) return
|
||||
|
||||
node.has_errors = true
|
||||
markNodeSlotErrors(node, nodeError)
|
||||
|
||||
for (const parentId of getParentExecutionIds(executionId)) {
|
||||
const parentNode = getNodeByExecutionId(rootGraph, parentId)
|
||||
if (parentNode) parentNode.has_errors = true
|
||||
}
|
||||
}
|
||||
|
||||
/** Execution error state: node errors, runtime errors, prompt errors, and missing nodes. */
|
||||
export const useExecutionErrorStore = defineStore('executionError', () => {
|
||||
const workflowStore = useWorkflowStore()
|
||||
const canvasStore = useCanvasStore()
|
||||
@@ -70,7 +105,7 @@ export const useExecutionErrorStore = defineStore('executionError', () => {
|
||||
lastPromptError.value = null
|
||||
}
|
||||
|
||||
/** Set missing node types detected during workflow load (deduplicated by nodeId, or by type for legacy string entries). */
|
||||
/** Deduplicates by nodeId for object entries, or by type string for legacy entries. */
|
||||
function setMissingNodeTypes(types: MissingNodeType[]) {
|
||||
if (!types.length) {
|
||||
missingNodesError.value = null
|
||||
@@ -171,7 +206,7 @@ export const useExecutionErrorStore = defineStore('executionError', () => {
|
||||
/** Count of runtime execution errors (0 or 1) */
|
||||
const executionErrorCount = computed(() => (lastExecutionError.value ? 1 : 0))
|
||||
|
||||
/** Count of missing node errors (0 or 1 — all missing nodes are a single error group) */
|
||||
/** Count of missing node errors (0 or 1) */
|
||||
const missingNodeCount = computed(() => (missingNodesError.value ? 1 : 0))
|
||||
|
||||
/** Total count of all individual errors */
|
||||
@@ -183,7 +218,7 @@ export const useExecutionErrorStore = defineStore('executionError', () => {
|
||||
missingNodeCount.value
|
||||
)
|
||||
|
||||
/** Pre-computed Set of graph node IDs (as strings) that have errors in the current graph scope. */
|
||||
/** Graph node IDs (as strings) that have errors in the current graph scope. */
|
||||
const activeGraphErrorNodeIds = computed<Set<string>>(() => {
|
||||
const ids = new Set<string>()
|
||||
if (!app.rootGraph) return ids
|
||||
@@ -211,28 +246,41 @@ export const useExecutionErrorStore = defineStore('executionError', () => {
|
||||
return ids
|
||||
})
|
||||
|
||||
const activeMissingNodeGraphIds = computed<Set<string>>(() => {
|
||||
const ids = new Set<string>()
|
||||
/**
|
||||
* Set of all execution ID prefixes derived from missing node execution IDs,
|
||||
* including the missing nodes themselves.
|
||||
*
|
||||
* Example: missing node at "65:70:63" → Set { "65", "65:70", "65:70:63" }
|
||||
*/
|
||||
const missingAncestorExecutionIds = computed<Set<NodeExecutionId>>(() => {
|
||||
const ids = new Set<NodeExecutionId>()
|
||||
const error = missingNodesError.value
|
||||
if (!error || !app.rootGraph) return ids
|
||||
|
||||
const activeGraph = canvasStore.currentGraph ?? app.rootGraph
|
||||
if (!error) return ids
|
||||
|
||||
for (const nodeType of error.nodeTypes) {
|
||||
if (typeof nodeType === 'string') continue
|
||||
if (nodeType.nodeId == null) continue
|
||||
const executionId = String(nodeType.nodeId)
|
||||
for (const id of getAncestorExecutionIds(String(nodeType.nodeId))) {
|
||||
ids.add(id)
|
||||
}
|
||||
}
|
||||
|
||||
return ids
|
||||
})
|
||||
|
||||
const activeMissingNodeGraphIds = computed<Set<string>>(() => {
|
||||
const ids = new Set<string>()
|
||||
if (!app.rootGraph) return ids
|
||||
|
||||
const activeGraph = canvasStore.currentGraph ?? app.rootGraph
|
||||
|
||||
for (const executionId of missingAncestorExecutionIds.value) {
|
||||
const graphNode = getNodeByExecutionId(app.rootGraph, executionId)
|
||||
if (graphNode?.graph === activeGraph) {
|
||||
ids.add(String(graphNode.id))
|
||||
}
|
||||
|
||||
const rootParent = getRootParentNode(app.rootGraph, executionId)
|
||||
if (rootParent) {
|
||||
ids.add(String(rootParent.id))
|
||||
}
|
||||
}
|
||||
|
||||
return ids
|
||||
})
|
||||
|
||||
@@ -282,15 +330,11 @@ export const useExecutionErrorStore = defineStore('executionError', () => {
|
||||
*/
|
||||
const errorAncestorExecutionIds = computed<Set<NodeExecutionId>>(() => {
|
||||
const ids = new Set<NodeExecutionId>()
|
||||
|
||||
for (const executionId of allErrorExecutionIds.value) {
|
||||
const parts = executionId.split(':')
|
||||
// Add every prefix including the full ID (error leaf node itself)
|
||||
for (let i = 1; i <= parts.length; i++) {
|
||||
ids.add(parts.slice(0, i).join(':'))
|
||||
for (const id of getAncestorExecutionIds(executionId)) {
|
||||
ids.add(id)
|
||||
}
|
||||
}
|
||||
|
||||
return ids
|
||||
})
|
||||
|
||||
@@ -302,59 +346,26 @@ export const useExecutionErrorStore = defineStore('executionError', () => {
|
||||
return errorAncestorExecutionIds.value.has(execId)
|
||||
}
|
||||
|
||||
/**
|
||||
* Update node and slot error flags when validation errors change.
|
||||
* Propagates errors up subgraph chains.
|
||||
*/
|
||||
watch(lastNodeErrors, () => {
|
||||
if (!app.rootGraph) return
|
||||
/** True if the node has a missing node inside it at any nesting depth. */
|
||||
function isContainerWithMissingNode(node: LGraphNode): boolean {
|
||||
if (!app.rootGraph) return false
|
||||
const execId = getExecutionIdByNode(app.rootGraph, node)
|
||||
if (!execId) return false
|
||||
return missingAncestorExecutionIds.value.has(execId)
|
||||
}
|
||||
|
||||
// Clear all error flags
|
||||
forEachNode(app.rootGraph, (node) => {
|
||||
node.has_errors = false
|
||||
if (node.inputs) {
|
||||
for (const slot of node.inputs) {
|
||||
slot.hasErrors = false
|
||||
}
|
||||
}
|
||||
})
|
||||
watch(lastNodeErrors, () => {
|
||||
const rootGraph = app.rootGraph
|
||||
if (!rootGraph) return
|
||||
|
||||
clearAllNodeErrorFlags(rootGraph)
|
||||
|
||||
if (!lastNodeErrors.value) return
|
||||
|
||||
// Set error flags on nodes and slots
|
||||
for (const [executionId, nodeError] of Object.entries(
|
||||
lastNodeErrors.value
|
||||
)) {
|
||||
const node = getNodeByExecutionId(app.rootGraph, executionId)
|
||||
if (!node) continue
|
||||
|
||||
node.has_errors = true
|
||||
|
||||
// Mark input slots with errors
|
||||
if (node.inputs) {
|
||||
for (const error of nodeError.errors) {
|
||||
const slotName = error.extra_info?.input_name
|
||||
if (!slotName) continue
|
||||
|
||||
const slot = node.inputs.find((s) => s.name === slotName)
|
||||
if (slot) {
|
||||
slot.hasErrors = true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Propagate errors to parent subgraph nodes
|
||||
const parts = executionId.split(':')
|
||||
for (let i = parts.length - 1; i > 0; i--) {
|
||||
const parentExecutionId = parts.slice(0, i).join(':')
|
||||
const parentNode = getNodeByExecutionId(
|
||||
app.rootGraph,
|
||||
parentExecutionId
|
||||
)
|
||||
if (parentNode) {
|
||||
parentNode.has_errors = true
|
||||
}
|
||||
}
|
||||
applyNodeError(rootGraph, executionId, nodeError)
|
||||
}
|
||||
})
|
||||
|
||||
@@ -393,6 +404,8 @@ export const useExecutionErrorStore = defineStore('executionError', () => {
|
||||
getNodeErrors,
|
||||
slotHasError,
|
||||
errorAncestorExecutionIds,
|
||||
isContainerWithInternalError
|
||||
isContainerWithInternalError,
|
||||
missingAncestorExecutionIds,
|
||||
isContainerWithMissingNode
|
||||
}
|
||||
})
|
||||
|
||||
61
src/utils/nodeExecutionUtil.ts
Normal file
61
src/utils/nodeExecutionUtil.ts
Normal file
@@ -0,0 +1,61 @@
|
||||
import type { NodeExecutionId } from '@/types/nodeIdentification'
|
||||
import type { ComfyNode } from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import { isSubgraphDefinition } from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
|
||||
/**
|
||||
* Returns all ancestor execution IDs for a given execution ID, including itself.
|
||||
*
|
||||
* Example: "65:70:63" → ["65", "65:70", "65:70:63"]
|
||||
*/
|
||||
export function getAncestorExecutionIds(
|
||||
executionId: string | NodeExecutionId
|
||||
): NodeExecutionId[] {
|
||||
const parts = executionId.split(':')
|
||||
return Array.from({ length: parts.length }, (_, i) =>
|
||||
parts.slice(0, i + 1).join(':')
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns all ancestor execution IDs for a given execution ID, excluding itself.
|
||||
*
|
||||
* Example: "65:70:63" → ["65", "65:70"]
|
||||
*/
|
||||
export function getParentExecutionIds(
|
||||
executionId: string | NodeExecutionId
|
||||
): NodeExecutionId[] {
|
||||
return getAncestorExecutionIds(executionId).slice(0, -1)
|
||||
}
|
||||
|
||||
/** "def-A" → ["5", "10"] for each container node instantiating that subgraph definition. */
|
||||
export function buildSubgraphExecutionPaths(
|
||||
rootNodes: ComfyNode[],
|
||||
allSubgraphDefs: unknown[]
|
||||
): Map<string, string[]> {
|
||||
const subgraphDefIds = new Set(
|
||||
allSubgraphDefs.filter(isSubgraphDefinition).map((s) => s.id)
|
||||
)
|
||||
const pathMap = new Map<string, string[]>()
|
||||
|
||||
const build = (nodes: ComfyNode[], parentPrefix: string) => {
|
||||
for (const n of nodes ?? []) {
|
||||
if (typeof n.type !== 'string' || !subgraphDefIds.has(n.type)) continue
|
||||
const path = parentPrefix ? `${parentPrefix}:${n.id}` : String(n.id)
|
||||
const existing = pathMap.get(n.type)
|
||||
if (existing) {
|
||||
existing.push(path)
|
||||
} else {
|
||||
pathMap.set(n.type, [path])
|
||||
}
|
||||
const innerDef = allSubgraphDefs.find(
|
||||
(s) => isSubgraphDefinition(s) && s.id === n.type
|
||||
)
|
||||
if (innerDef && isSubgraphDefinition(innerDef)) {
|
||||
build(innerDef.nodes, path)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
build(rootNodes, '')
|
||||
return pathMap
|
||||
}
|
||||
@@ -36,7 +36,7 @@ export function usePackInstall(
|
||||
|
||||
const createPayload = (installItem: NodePack) => {
|
||||
if (!installItem.id) {
|
||||
throw new Error('Node ID is required for installation')
|
||||
throw new Error(t('manager.packInstall.nodeIdRequired'))
|
||||
}
|
||||
|
||||
const isUnclaimedPack = installItem.publisher?.name === 'Unclaimed'
|
||||
|
||||
Reference in New Issue
Block a user