mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-24 16:54:03 +00:00
fix: fix error overlay and TabErrors filtering for nested subgraphs (#9129)
## Summary Fix error overlays not showing on subgraph container nodes and nested error cards not appearing in the Errors tab when a node inside a subgraph fails. ## Changes - **What**: Error overlay and Errors tab filtering now use full hierarchical execution IDs (e.g. `65:70`) instead of local node IDs, enabling correct ancestor detection at any nesting depth - Added `getExecutionIdByNode` to [graphTraversalUtil.ts](src/utils/graphTraversalUtil.ts) to compute a node's full execution ID chain from the root graph - Added `errorAncestorExecutionIds` computed set and `isContainerWithInternalError(node)` helper to [executionErrorStore.ts](src/stores/executionErrorStore.ts) for O(1) container checks - Updated `hasAnyError` in [LGraphNode.vue](src/extensions/vueNodes/components/LGraphNode.vue) to use the new store helper - Fixed `isErrorInSelection` in [useErrorGroups.ts](src/components/rightSidePanel/errors/useErrorGroups.ts) to use full execution IDs for selected containers ## Review Focus - `errorAncestorExecutionIds` is rebuilt reactively whenever the active errors change — confirm this is efficient enough given typical error counts - `getExecutionIdByNode` walks up the graph hierarchy; verify the base case (root graph, no parent) is handled correctly ## Screenshots https://github.com/user-attachments/assets/b5be5892-80a9-4e5e-8b6f-fe754b4ebc4e https://github.com/user-attachments/assets/92ff12b3-3bc9-4f02-ba4a-e2c7384bafe5 https://github.com/user-attachments/assets/be8e95be-ac8c-4699-9be9-b11902294bda ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9129-fix-fix-error-overlay-and-TabErrors-filtering-for-nested-subgraphs-3106d73d365081c1875bc1a3c89eae29) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -106,7 +106,7 @@ const hasContainerInternalError = computed(() => {
|
||||
if (allErrorExecutionIds.value.length === 0) return false
|
||||
return selectedNodes.value.some((node) => {
|
||||
if (!(node instanceof SubgraphNode || isGroupNode(node))) return false
|
||||
return executionErrorStore.hasInternalErrorForNode(node.id)
|
||||
return executionErrorStore.isContainerWithInternalError(node)
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -13,6 +13,7 @@ import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
|
||||
import {
|
||||
getNodeByExecutionId,
|
||||
getExecutionIdByNode,
|
||||
getRootParentNode
|
||||
} from '@/utils/graphTraversalUtil'
|
||||
import { resolveNodeDisplayName } from '@/utils/nodeTitleUtil'
|
||||
@@ -20,6 +21,7 @@ import { isLGraphNode } from '@/utils/litegraphUtil'
|
||||
import { isGroupNode } from '@/utils/executableGroupNodeDto'
|
||||
import { st } from '@/i18n'
|
||||
import type { ErrorCardData, ErrorGroup, ErrorItem } from './types'
|
||||
import type { NodeExecutionId } from '@/types/nodeIdentification'
|
||||
import { isNodeExecutionId } from '@/types/nodeIdentification'
|
||||
|
||||
const PROMPT_CARD_ID = '__prompt__'
|
||||
@@ -200,26 +202,30 @@ export function useErrorGroups(
|
||||
const selectedNodeInfo = computed(() => {
|
||||
const items = canvasStore.selectedItems
|
||||
const nodeIds = new Set<string>()
|
||||
const containerIds = new Set<string>()
|
||||
const containerExecutionIds = new Set<NodeExecutionId>()
|
||||
|
||||
for (const item of items) {
|
||||
if (!isLGraphNode(item)) continue
|
||||
nodeIds.add(String(item.id))
|
||||
if (item instanceof SubgraphNode || isGroupNode(item)) {
|
||||
containerIds.add(String(item.id))
|
||||
if (
|
||||
(item instanceof SubgraphNode || isGroupNode(item)) &&
|
||||
app.rootGraph
|
||||
) {
|
||||
const execId = getExecutionIdByNode(app.rootGraph, item)
|
||||
if (execId) containerExecutionIds.add(execId)
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
nodeIds: nodeIds.size > 0 ? nodeIds : null,
|
||||
containerIds
|
||||
containerExecutionIds
|
||||
}
|
||||
})
|
||||
|
||||
const isSingleNodeSelected = computed(
|
||||
() =>
|
||||
selectedNodeInfo.value.nodeIds?.size === 1 &&
|
||||
selectedNodeInfo.value.containerIds.size === 0
|
||||
selectedNodeInfo.value.containerExecutionIds.size === 0
|
||||
)
|
||||
|
||||
const errorNodeCache = computed(() => {
|
||||
@@ -238,8 +244,9 @@ export function useErrorGroups(
|
||||
const graphNode = errorNodeCache.value.get(executionNodeId)
|
||||
if (graphNode && nodeIds.has(String(graphNode.id))) return true
|
||||
|
||||
for (const containerId of selectedNodeInfo.value.containerIds) {
|
||||
if (executionNodeId.startsWith(`${containerId}:`)) return true
|
||||
for (const containerExecId of selectedNodeInfo.value
|
||||
.containerExecutionIds) {
|
||||
if (executionNodeId.startsWith(`${containerExecId}:`)) return true
|
||||
}
|
||||
|
||||
return false
|
||||
|
||||
@@ -121,7 +121,7 @@ const hasContainerInternalError = computed(() => {
|
||||
targetNode.value instanceof SubgraphNode || isGroupNode(targetNode.value)
|
||||
if (!isContainer) return false
|
||||
|
||||
return executionErrorStore.hasInternalErrorForNode(targetNode.value.id)
|
||||
return executionErrorStore.isContainerWithInternalError(targetNode.value)
|
||||
})
|
||||
|
||||
const nodeHasError = computed(() => {
|
||||
|
||||
@@ -342,7 +342,9 @@ const hasAnyError = computed((): boolean => {
|
||||
hasExecutionError.value ||
|
||||
nodeData.hasErrors ||
|
||||
error ||
|
||||
(executionErrorStore.lastNodeErrors?.[nodeData.id]?.errors.length ?? 0) > 0
|
||||
executionErrorStore.getNodeErrors(nodeLocatorId.value) ||
|
||||
(lgraphNode.value &&
|
||||
executionErrorStore.isContainerWithInternalError(lgraphNode.value))
|
||||
)
|
||||
})
|
||||
|
||||
|
||||
@@ -10,11 +10,13 @@ import type {
|
||||
PromptError
|
||||
} from '@/schemas/apiSchema'
|
||||
import type { NodeId } from '@/platform/workflow/validation/schemas/workflowSchema'
|
||||
import type { NodeLocatorId } from '@/types/nodeIdentification'
|
||||
import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import type { NodeExecutionId, NodeLocatorId } from '@/types/nodeIdentification'
|
||||
import {
|
||||
executionIdToNodeLocatorId,
|
||||
forEachNode,
|
||||
getNodeByExecutionId
|
||||
getNodeByExecutionId,
|
||||
getExecutionIdByNode
|
||||
} from '@/utils/graphTraversalUtil'
|
||||
|
||||
/**
|
||||
@@ -186,9 +188,32 @@ export const useExecutionErrorStore = defineStore('executionError', () => {
|
||||
return nodeError.errors.some((e) => e.extra_info?.input_name === slotName)
|
||||
}
|
||||
|
||||
function hasInternalErrorForNode(nodeId: string | number): boolean {
|
||||
const prefix = `${nodeId}:`
|
||||
return allErrorExecutionIds.value.some((id) => id.startsWith(prefix))
|
||||
/**
|
||||
* Set of all execution ID prefixes derived from active error nodes,
|
||||
* including the error nodes themselves.
|
||||
*
|
||||
* Example: error at "65:70:63" → Set { "65", "65:70", "65:70:63" }
|
||||
*/
|
||||
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(':'))
|
||||
}
|
||||
}
|
||||
|
||||
return ids
|
||||
})
|
||||
|
||||
/** True if the node has errors inside it at any nesting depth. */
|
||||
function isContainerWithInternalError(node: LGraphNode): boolean {
|
||||
if (!app.rootGraph) return false
|
||||
const execId = getExecutionIdByNode(app.rootGraph, node)
|
||||
if (!execId) return false
|
||||
return errorAncestorExecutionIds.value.has(execId)
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -275,6 +300,7 @@ export const useExecutionErrorStore = defineStore('executionError', () => {
|
||||
// Lookup helpers
|
||||
getNodeErrors,
|
||||
slotHasError,
|
||||
hasInternalErrorForNode
|
||||
errorAncestorExecutionIds,
|
||||
isContainerWithInternalError
|
||||
}
|
||||
})
|
||||
|
||||
@@ -331,6 +331,35 @@ export function getNodeByExecutionId(
|
||||
return targetGraph.getNodeById(localNodeId) || null
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the execution ID for a node relative to the root graph.
|
||||
*
|
||||
* Root-level nodes return their ID directly (e.g. "42").
|
||||
* Nodes inside subgraphs return a colon-separated chain (e.g. "65:70:63").
|
||||
*
|
||||
* @param rootGraph - The root graph to resolve from
|
||||
* @param node - The node whose execution ID to compute
|
||||
* @returns The execution ID string, or null if the node has no graph
|
||||
*/
|
||||
export function getExecutionIdByNode(
|
||||
rootGraph: LGraph,
|
||||
node: LGraphNode
|
||||
): NodeExecutionId | null {
|
||||
if (!node.graph) return null
|
||||
|
||||
if (node.graph === rootGraph || node.graph.isRootGraph) {
|
||||
return String(node.id)
|
||||
}
|
||||
|
||||
const parentPath = findPartialExecutionPathToGraph(
|
||||
node.graph as LGraph,
|
||||
rootGraph
|
||||
)
|
||||
if (parentPath === undefined) return null
|
||||
|
||||
return `${parentPath}:${node.id}`
|
||||
}
|
||||
|
||||
/**
|
||||
* Get a node by its locator ID from anywhere in the graph hierarchy.
|
||||
* Locator IDs use UUID format like "uuid:nodeId" for subgraph nodes.
|
||||
|
||||
Reference in New Issue
Block a user