mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-01-26 19:09:52 +00:00
feat: propagate errors up subgraphs and show slot errors in subgraphs (#6963)
https://github.com/user-attachments/assets/6531879d-a8a2-420a-aaca-ee329386dd1a ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6963-feat-propagate-errors-up-subgraphs-and-show-slot-errors-in-subgraphs-2b76d73d3650813e8391fac0a5e6dc9b) by [Unito](https://www.unito.io) --------- Co-authored-by: github-actions <github-actions@github.com>
This commit is contained in:
@@ -1,13 +1,13 @@
|
||||
// Import only English locale eagerly as the default/fallback
|
||||
// ESLint cannot statically resolve dynamic imports with path aliases (@frontend-locales/*),
|
||||
// but these are properly configured in tsconfig.json and resolved by Vite at build time.
|
||||
// eslint-disable-next-line import-x/no-unresolved
|
||||
|
||||
import enCommands from '@frontend-locales/en/commands.json' with { type: 'json' }
|
||||
// eslint-disable-next-line import-x/no-unresolved
|
||||
|
||||
import en from '@frontend-locales/en/main.json' with { type: 'json' }
|
||||
// eslint-disable-next-line import-x/no-unresolved
|
||||
|
||||
import enNodes from '@frontend-locales/en/nodeDefs.json' with { type: 'json' }
|
||||
// eslint-disable-next-line import-x/no-unresolved
|
||||
|
||||
import enSettings from '@frontend-locales/en/settings.json' with { type: 'json' }
|
||||
import { createI18n } from 'vue-i18n'
|
||||
|
||||
@@ -27,7 +27,7 @@ function buildLocale<
|
||||
|
||||
// Locale loader map - dynamically import locales only when needed
|
||||
// ESLint cannot statically resolve these dynamic imports, but they are valid at build time
|
||||
/* eslint-disable import-x/no-unresolved */
|
||||
|
||||
const localeLoaders: Record<
|
||||
string,
|
||||
() => Promise<{ default: Record<string, unknown> }>
|
||||
|
||||
Binary file not shown.
|
Before Width: | Height: | Size: 95 KiB After Width: | Height: | Size: 97 KiB |
@@ -119,11 +119,6 @@ export const useLitegraphService = () => {
|
||||
return { color: '#0f0' }
|
||||
}
|
||||
}
|
||||
node.strokeStyles['nodeError'] = function (this: LGraphNode) {
|
||||
if (app.lastNodeErrors?.[this.id]?.errors) {
|
||||
return { color: 'red' }
|
||||
}
|
||||
}
|
||||
node.strokeStyles['dragOver'] = function (this: LGraphNode) {
|
||||
if (app.dragOverNode?.id == this.id) {
|
||||
return { color: 'dodgerblue' }
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { defineStore } from 'pinia'
|
||||
import { computed, ref } from 'vue'
|
||||
import { computed, ref, watch } from 'vue'
|
||||
|
||||
import { useNodeProgressText } from '@/composables/node/useNodeProgressText'
|
||||
import type { LGraph, Subgraph } from '@/lib/litegraph/src/litegraph'
|
||||
@@ -32,6 +32,7 @@ import { app } from '@/scripts/app'
|
||||
import { useNodeOutputStore } from '@/stores/imagePreviewStore'
|
||||
import type { NodeLocatorId } from '@/types/nodeIdentification'
|
||||
import { createNodeLocatorId } from '@/types/nodeIdentification'
|
||||
import { forEachNode, getNodeByExecutionId } from '@/utils/graphTraversalUtil'
|
||||
|
||||
interface QueuedPrompt {
|
||||
/**
|
||||
@@ -534,6 +535,97 @@ export const useExecutionStore = defineStore('execution', () => {
|
||||
() => runningPromptIds.value.length
|
||||
)
|
||||
|
||||
/** Map of node errors indexed by locator ID. */
|
||||
const nodeErrorsByLocatorId = computed<Record<NodeLocatorId, NodeError>>(
|
||||
() => {
|
||||
if (!lastNodeErrors.value) return {}
|
||||
|
||||
const map: Record<NodeLocatorId, NodeError> = {}
|
||||
|
||||
for (const [executionId, nodeError] of Object.entries(
|
||||
lastNodeErrors.value
|
||||
)) {
|
||||
const locatorId = executionIdToNodeLocatorId(executionId)
|
||||
if (locatorId) {
|
||||
map[locatorId] = nodeError
|
||||
}
|
||||
}
|
||||
|
||||
return map
|
||||
}
|
||||
)
|
||||
|
||||
/** Get node errors by locator ID. */
|
||||
const getNodeErrors = (
|
||||
nodeLocatorId: NodeLocatorId
|
||||
): NodeError | undefined => {
|
||||
return nodeErrorsByLocatorId.value[nodeLocatorId]
|
||||
}
|
||||
|
||||
/** Check if a specific slot has validation errors. */
|
||||
const slotHasError = (
|
||||
nodeLocatorId: NodeLocatorId,
|
||||
slotName: string
|
||||
): boolean => {
|
||||
const nodeError = getNodeErrors(nodeLocatorId)
|
||||
if (!nodeError) return false
|
||||
|
||||
return nodeError.errors.some((e) => e.extra_info?.input_name === slotName)
|
||||
}
|
||||
|
||||
/**
|
||||
* Update node and slot error flags when validation errors change.
|
||||
* Propagates errors up subgraph chains.
|
||||
*/
|
||||
watch(lastNodeErrors, () => {
|
||||
if (!app.graph || !app.graph.nodes) return
|
||||
|
||||
// Clear all error flags
|
||||
forEachNode(app.graph, (node) => {
|
||||
node.has_errors = false
|
||||
if (node.inputs) {
|
||||
for (const slot of node.inputs) {
|
||||
slot.hasErrors = false
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
if (!lastNodeErrors.value) return
|
||||
|
||||
// Set error flags on nodes and slots
|
||||
for (const [executionId, nodeError] of Object.entries(
|
||||
lastNodeErrors.value
|
||||
)) {
|
||||
const node = getNodeByExecutionId(app.graph, 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.graph, parentExecutionId)
|
||||
if (parentNode) {
|
||||
parentNode.has_errors = true
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
return {
|
||||
isIdle,
|
||||
clientId,
|
||||
@@ -567,6 +659,9 @@ export const useExecutionStore = defineStore('execution', () => {
|
||||
// NodeLocatorId conversion helpers
|
||||
executionIdToNodeLocatorId,
|
||||
nodeLocatorIdToExecutionId,
|
||||
promptIdToWorkflowId
|
||||
promptIdToWorkflowId,
|
||||
// Node error lookup helpers
|
||||
getNodeErrors,
|
||||
slotHasError
|
||||
}
|
||||
})
|
||||
|
||||
@@ -129,3 +129,170 @@ describe('useExecutionStore - NodeLocatorId conversions', () => {
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('useExecutionStore - Node Error Lookups', () => {
|
||||
let store: ReturnType<typeof useExecutionStore>
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
setActivePinia(createPinia())
|
||||
store = useExecutionStore()
|
||||
})
|
||||
|
||||
describe('getNodeErrors', () => {
|
||||
it('should return undefined when no errors exist', () => {
|
||||
const result = store.getNodeErrors('123')
|
||||
expect(result).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should return node error by locator ID for root graph node', () => {
|
||||
store.lastNodeErrors = {
|
||||
'123': {
|
||||
errors: [
|
||||
{
|
||||
type: 'validation_error',
|
||||
message: 'Invalid input',
|
||||
details: 'Width must be positive',
|
||||
extra_info: { input_name: 'width' }
|
||||
}
|
||||
],
|
||||
class_type: 'TestNode',
|
||||
dependent_outputs: []
|
||||
}
|
||||
}
|
||||
|
||||
const result = store.getNodeErrors('123')
|
||||
expect(result).toBeDefined()
|
||||
expect(result?.errors).toHaveLength(1)
|
||||
expect(result?.errors[0].message).toBe('Invalid input')
|
||||
})
|
||||
|
||||
it('should return node error by locator ID for subgraph node', () => {
|
||||
const subgraphUuid = 'a1b2c3d4-e5f6-7890-abcd-ef1234567890'
|
||||
const mockSubgraph = {
|
||||
id: subgraphUuid,
|
||||
_nodes: []
|
||||
}
|
||||
|
||||
const mockNode = {
|
||||
id: 123,
|
||||
isSubgraphNode: () => true,
|
||||
subgraph: mockSubgraph
|
||||
} as any
|
||||
|
||||
vi.mocked(app.graph.getNodeById).mockReturnValue(mockNode)
|
||||
|
||||
store.lastNodeErrors = {
|
||||
'123:456': {
|
||||
errors: [
|
||||
{
|
||||
type: 'validation_error',
|
||||
message: 'Invalid subgraph input',
|
||||
details: 'Missing required input',
|
||||
extra_info: { input_name: 'image' }
|
||||
}
|
||||
],
|
||||
class_type: 'SubgraphNode',
|
||||
dependent_outputs: []
|
||||
}
|
||||
}
|
||||
|
||||
const locatorId = `${subgraphUuid}:456`
|
||||
const result = store.getNodeErrors(locatorId)
|
||||
expect(result).toBeDefined()
|
||||
expect(result?.errors[0].message).toBe('Invalid subgraph input')
|
||||
})
|
||||
})
|
||||
|
||||
describe('slotHasError', () => {
|
||||
it('should return false when node has no errors', () => {
|
||||
const result = store.slotHasError('123', 'width')
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
|
||||
it('should return false when node has errors but slot is not mentioned', () => {
|
||||
store.lastNodeErrors = {
|
||||
'123': {
|
||||
errors: [
|
||||
{
|
||||
type: 'validation_error',
|
||||
message: 'Invalid input',
|
||||
details: 'Width must be positive',
|
||||
extra_info: { input_name: 'width' }
|
||||
}
|
||||
],
|
||||
class_type: 'TestNode',
|
||||
dependent_outputs: []
|
||||
}
|
||||
}
|
||||
|
||||
const result = store.slotHasError('123', 'height')
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
|
||||
it('should return true when slot has error', () => {
|
||||
store.lastNodeErrors = {
|
||||
'123': {
|
||||
errors: [
|
||||
{
|
||||
type: 'validation_error',
|
||||
message: 'Invalid input',
|
||||
details: 'Width must be positive',
|
||||
extra_info: { input_name: 'width' }
|
||||
}
|
||||
],
|
||||
class_type: 'TestNode',
|
||||
dependent_outputs: []
|
||||
}
|
||||
}
|
||||
|
||||
const result = store.slotHasError('123', 'width')
|
||||
expect(result).toBe(true)
|
||||
})
|
||||
|
||||
it('should return true when multiple errors exist for the same slot', () => {
|
||||
store.lastNodeErrors = {
|
||||
'123': {
|
||||
errors: [
|
||||
{
|
||||
type: 'validation_error',
|
||||
message: 'Invalid input',
|
||||
details: 'Width must be positive',
|
||||
extra_info: { input_name: 'width' }
|
||||
},
|
||||
{
|
||||
type: 'validation_error',
|
||||
message: 'Invalid range',
|
||||
details: 'Width must be less than 1000',
|
||||
extra_info: { input_name: 'width' }
|
||||
}
|
||||
],
|
||||
class_type: 'TestNode',
|
||||
dependent_outputs: []
|
||||
}
|
||||
}
|
||||
|
||||
const result = store.slotHasError('123', 'width')
|
||||
expect(result).toBe(true)
|
||||
})
|
||||
|
||||
it('should handle errors without extra_info', () => {
|
||||
store.lastNodeErrors = {
|
||||
'123': {
|
||||
errors: [
|
||||
{
|
||||
type: 'validation_error',
|
||||
message: 'General error',
|
||||
details: 'Something went wrong'
|
||||
}
|
||||
],
|
||||
class_type: 'TestNode',
|
||||
dependent_outputs: []
|
||||
}
|
||||
}
|
||||
|
||||
const result = store.slotHasError('123', 'width')
|
||||
expect(result).toBe(false)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user