mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-13 09:00:16 +00:00
## Summary Surfaces missing node pack information in the Errors Tab, grouped by registry pack, with one-click install support via ComfyUI Manager. ## Changes - **What**: Errors Tab now groups missing nodes by their registry pack and shows a `MissingPackGroupRow` with pack name, node/pack counts, and an Install button that triggers Manager installation. A `MissingNodeCard` shows individual unresolvable nodes that have no associated pack. `useErrorGroups` was extended to resolve missing node types to their registry packs using the `/api/workflow/missing_nodes` endpoint. `executionErrorStore` was refactored to track missing node types separately from execution errors and expose them reactively. - **Breaking**: None ## Review Focus - `useErrorGroups.ts` — the new `resolveMissingNodePacks` logic fetches pack metadata and maps node types to pack IDs; edge cases around partial resolution (some nodes have a pack, some don't) produce both `MissingPackGroupRow` and `MissingNodeCard` entries - `executionErrorStore.ts` — the store now separates `missingNodeTypes` state from `errors`; the deferred-warnings path in `app.ts` now calls `setMissingNodeTypes` so the Errors Tab is populated even when a workflow loads without executing ## Screenshots (if applicable) https://github.com/user-attachments/assets/97f8d009-0cac-4739-8740-fd3333b5a85b ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9213-feat-show-missing-node-packs-in-Errors-Tab-with-install-support-3126d73d36508197bc4bf8ebfd2125c8) by [Unito](https://www.unito.io)
445 lines
14 KiB
TypeScript
445 lines
14 KiB
TypeScript
import { setActivePinia } from 'pinia'
|
|
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
|
import { app } from '@/scripts/app'
|
|
import { useExecutionStore } from '@/stores/executionStore'
|
|
import { useExecutionErrorStore } from '@/stores/executionErrorStore'
|
|
import { executionIdToNodeLocatorId } from '@/utils/graphTraversalUtil'
|
|
|
|
// Create mock functions that will be shared
|
|
const mockNodeExecutionIdToNodeLocatorId = vi.fn()
|
|
const mockNodeIdToNodeLocatorId = vi.fn()
|
|
const mockNodeLocatorIdToNodeExecutionId = vi.fn()
|
|
|
|
import type * as WorkflowStoreModule from '@/platform/workflow/management/stores/workflowStore'
|
|
import { createMockLGraphNode } from '@/utils/__tests__/litegraphTestUtils'
|
|
import { createTestingPinia } from '@pinia/testing'
|
|
|
|
// Mock the workflowStore
|
|
vi.mock('@/platform/workflow/management/stores/workflowStore', async () => {
|
|
const { ComfyWorkflow } = await vi.importActual<typeof WorkflowStoreModule>(
|
|
'@/platform/workflow/management/stores/workflowStore'
|
|
)
|
|
return {
|
|
ComfyWorkflow,
|
|
useWorkflowStore: vi.fn(() => ({
|
|
nodeExecutionIdToNodeLocatorId: mockNodeExecutionIdToNodeLocatorId,
|
|
nodeIdToNodeLocatorId: mockNodeIdToNodeLocatorId,
|
|
nodeLocatorIdToNodeExecutionId: mockNodeLocatorIdToNodeExecutionId
|
|
}))
|
|
}
|
|
})
|
|
|
|
// Remove any previous global types
|
|
declare global {
|
|
interface Window {}
|
|
}
|
|
|
|
vi.mock('@/composables/node/useNodeProgressText', () => ({
|
|
useNodeProgressText: () => ({
|
|
showTextPreview: vi.fn()
|
|
})
|
|
}))
|
|
|
|
// Mock the app import with proper implementation
|
|
vi.mock('@/scripts/app', () => ({
|
|
app: {
|
|
rootGraph: {
|
|
getNodeById: vi.fn(),
|
|
nodes: [] // Add nodes array for workflowStore iteration
|
|
},
|
|
revokePreviews: vi.fn(),
|
|
nodePreviewImages: {}
|
|
}
|
|
}))
|
|
|
|
describe('useExecutionStore - NodeLocatorId conversions', () => {
|
|
let store: ReturnType<typeof useExecutionStore>
|
|
|
|
beforeEach(() => {
|
|
vi.clearAllMocks()
|
|
// Reset mock implementations
|
|
mockNodeExecutionIdToNodeLocatorId.mockReset()
|
|
mockNodeIdToNodeLocatorId.mockReset()
|
|
mockNodeLocatorIdToNodeExecutionId.mockReset()
|
|
|
|
setActivePinia(createTestingPinia({ stubActions: false }))
|
|
store = useExecutionStore()
|
|
})
|
|
|
|
describe('executionIdToNodeLocatorId', () => {
|
|
it('should convert execution ID to NodeLocatorId', () => {
|
|
// Mock subgraph structure
|
|
const mockSubgraph = {
|
|
id: 'a1b2c3d4-e5f6-7890-abcd-ef1234567890',
|
|
nodes: []
|
|
}
|
|
|
|
const mockNode = createMockLGraphNode({
|
|
id: 123,
|
|
isSubgraphNode: () => true,
|
|
subgraph: mockSubgraph
|
|
})
|
|
// Mock app.rootGraph.getNodeById to return the mock node
|
|
vi.mocked(app.rootGraph.getNodeById).mockReturnValue(mockNode)
|
|
|
|
const result = executionIdToNodeLocatorId(app.rootGraph, '123:456')
|
|
|
|
expect(result).toBe('a1b2c3d4-e5f6-7890-abcd-ef1234567890:456')
|
|
})
|
|
|
|
it('should convert simple node ID to NodeLocatorId', () => {
|
|
const result = executionIdToNodeLocatorId(app.rootGraph, '123')
|
|
|
|
// For simple node IDs, it should return the ID as-is
|
|
expect(result).toBe('123')
|
|
})
|
|
|
|
it('should handle numeric node IDs', () => {
|
|
const result = executionIdToNodeLocatorId(app.rootGraph, 123)
|
|
|
|
// For numeric IDs, it should convert to string and return as-is
|
|
expect(result).toBe('123')
|
|
})
|
|
|
|
it('should return undefined when conversion fails', () => {
|
|
// Mock app.rootGraph.getNodeById to return null (node not found)
|
|
vi.mocked(app.rootGraph.getNodeById).mockReturnValue(null)
|
|
|
|
expect(executionIdToNodeLocatorId(app.rootGraph, '999:456')).toBe(
|
|
undefined
|
|
)
|
|
})
|
|
})
|
|
|
|
describe('nodeLocatorIdToExecutionId', () => {
|
|
it('should convert NodeLocatorId to execution ID', () => {
|
|
const mockExecutionId = '123:456'
|
|
mockNodeLocatorIdToNodeExecutionId.mockReturnValue(mockExecutionId)
|
|
|
|
const result = store.nodeLocatorIdToExecutionId(
|
|
'a1b2c3d4-e5f6-7890-abcd-ef1234567890:456'
|
|
)
|
|
|
|
expect(mockNodeLocatorIdToNodeExecutionId).toHaveBeenCalledWith(
|
|
'a1b2c3d4-e5f6-7890-abcd-ef1234567890:456'
|
|
)
|
|
expect(result).toBe(mockExecutionId)
|
|
})
|
|
|
|
it('should return null when conversion fails', () => {
|
|
mockNodeLocatorIdToNodeExecutionId.mockReturnValue(null)
|
|
|
|
const result = store.nodeLocatorIdToExecutionId('invalid:format')
|
|
|
|
expect(result).toBeNull()
|
|
})
|
|
})
|
|
})
|
|
|
|
describe('useExecutionStore - reconcileInitializingJobs', () => {
|
|
let store: ReturnType<typeof useExecutionStore>
|
|
|
|
beforeEach(() => {
|
|
vi.clearAllMocks()
|
|
setActivePinia(createTestingPinia({ stubActions: false }))
|
|
store = useExecutionStore()
|
|
})
|
|
|
|
it('should remove job IDs not present in active jobs', () => {
|
|
store.initializingJobIds = new Set(['job-1', 'job-2', 'job-3'])
|
|
|
|
store.reconcileInitializingJobs(new Set(['job-1']))
|
|
|
|
expect(store.initializingJobIds).toEqual(new Set(['job-1']))
|
|
})
|
|
|
|
it('should be a no-op when all initializing IDs are active', () => {
|
|
store.initializingJobIds = new Set(['job-1', 'job-2'])
|
|
|
|
store.reconcileInitializingJobs(new Set(['job-1', 'job-2', 'job-3']))
|
|
|
|
expect(store.initializingJobIds).toEqual(new Set(['job-1', 'job-2']))
|
|
})
|
|
|
|
it('should be a no-op when there are no initializing jobs', () => {
|
|
store.initializingJobIds = new Set()
|
|
|
|
store.reconcileInitializingJobs(new Set(['job-1']))
|
|
|
|
expect(store.initializingJobIds).toEqual(new Set())
|
|
})
|
|
|
|
it('should clear all initializing IDs when no active jobs exist', () => {
|
|
store.initializingJobIds = new Set(['job-1', 'job-2'])
|
|
|
|
store.reconcileInitializingJobs(new Set())
|
|
|
|
expect(store.initializingJobIds).toEqual(new Set())
|
|
})
|
|
})
|
|
|
|
describe('useExecutionErrorStore - Node Error Lookups', () => {
|
|
let store: ReturnType<typeof useExecutionErrorStore>
|
|
|
|
beforeEach(() => {
|
|
vi.clearAllMocks()
|
|
setActivePinia(createTestingPinia({ stubActions: false }))
|
|
store = useExecutionErrorStore()
|
|
})
|
|
|
|
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,
|
|
getNodeById: vi.fn(),
|
|
nodes: []
|
|
}
|
|
|
|
const mockNode = createMockLGraphNode({
|
|
id: 123,
|
|
isSubgraphNode: () => true,
|
|
subgraph: mockSubgraph
|
|
})
|
|
|
|
vi.mocked(app.rootGraph.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)
|
|
})
|
|
})
|
|
})
|
|
|
|
describe('useExecutionErrorStore - setMissingNodeTypes', () => {
|
|
let store: ReturnType<typeof useExecutionErrorStore>
|
|
|
|
beforeEach(() => {
|
|
vi.clearAllMocks()
|
|
setActivePinia(createTestingPinia({ stubActions: false }))
|
|
store = useExecutionErrorStore()
|
|
})
|
|
|
|
it('clears missingNodesError when called with an empty array', () => {
|
|
store.setMissingNodeTypes([{ type: 'NodeA' }])
|
|
store.setMissingNodeTypes([])
|
|
expect(store.missingNodesError).toBeNull()
|
|
})
|
|
|
|
it('hasMissingNodes is false when error is null', () => {
|
|
store.setMissingNodeTypes([])
|
|
expect(store.hasMissingNodes).toBe(false)
|
|
})
|
|
|
|
it('hasMissingNodes is true after setting non-empty types', () => {
|
|
store.setMissingNodeTypes([{ type: 'NodeA' }])
|
|
expect(store.hasMissingNodes).toBe(true)
|
|
})
|
|
|
|
it('deduplicates string entries by value', () => {
|
|
store.setMissingNodeTypes(['GroupNode', 'GroupNode', 'OtherGroup'])
|
|
expect(store.missingNodesError?.nodeTypes).toHaveLength(2)
|
|
expect(store.missingNodesError?.nodeTypes).toEqual([
|
|
'GroupNode',
|
|
'OtherGroup'
|
|
])
|
|
})
|
|
|
|
it('keeps a single string entry unchanged', () => {
|
|
store.setMissingNodeTypes(['GroupNode'])
|
|
expect(store.missingNodesError?.nodeTypes).toHaveLength(1)
|
|
})
|
|
|
|
it('deduplicates object entries with the same nodeId', () => {
|
|
store.setMissingNodeTypes([
|
|
{ type: 'NodeA', nodeId: 1 },
|
|
{ type: 'NodeA', nodeId: 1 }
|
|
])
|
|
expect(store.missingNodesError?.nodeTypes).toHaveLength(1)
|
|
})
|
|
|
|
it('keeps object entries with different nodeIds even if same type', () => {
|
|
store.setMissingNodeTypes([
|
|
{ type: 'NodeA', nodeId: 1 },
|
|
{ type: 'NodeA', nodeId: 2 }
|
|
])
|
|
expect(store.missingNodesError?.nodeTypes).toHaveLength(2)
|
|
})
|
|
|
|
it('deduplicates object entries by type when nodeId is absent', () => {
|
|
store.setMissingNodeTypes([{ type: 'NodeB' }, { type: 'NodeB' }])
|
|
expect(store.missingNodesError?.nodeTypes).toHaveLength(1)
|
|
})
|
|
|
|
it('keeps distinct types when nodeId is absent', () => {
|
|
store.setMissingNodeTypes([{ type: 'NodeB' }, { type: 'NodeC' }])
|
|
expect(store.missingNodesError?.nodeTypes).toHaveLength(2)
|
|
})
|
|
|
|
it('treats absent nodeId the same as type-only key (falls back to type)', () => {
|
|
store.setMissingNodeTypes([{ type: 'NodeD' }, { type: 'NodeD' }])
|
|
expect(store.missingNodesError?.nodeTypes).toHaveLength(1)
|
|
})
|
|
|
|
it('handles a mix of string and object entries correctly', () => {
|
|
store.setMissingNodeTypes([
|
|
'GroupNode',
|
|
'GroupNode', // string dup
|
|
{ type: 'NodeA', nodeId: 1 },
|
|
{ type: 'NodeA', nodeId: 1 }, // object dup by nodeId
|
|
{ type: 'NodeA', nodeId: 2 }, // same type, different nodeId → kept
|
|
{ type: 'NodeB' },
|
|
{ type: 'NodeB' } // object dup by type
|
|
])
|
|
// Unique: 'GroupNode', {NodeA,1}, {NodeA,2}, {NodeB} → 4
|
|
expect(store.missingNodesError?.nodeTypes).toHaveLength(4)
|
|
})
|
|
|
|
it('stores a non-empty message string in missingNodesError', () => {
|
|
store.setMissingNodeTypes([{ type: 'NodeA' }])
|
|
expect(typeof store.missingNodesError?.message).toBe('string')
|
|
expect(store.missingNodesError!.message.length).toBeGreaterThan(0)
|
|
})
|
|
|
|
it('stores the deduplicated nodeTypes array in missingNodesError', () => {
|
|
const input = [{ type: 'NodeA' }, { type: 'NodeB' }]
|
|
store.setMissingNodeTypes(input)
|
|
expect(store.missingNodesError?.nodeTypes).toEqual(input)
|
|
})
|
|
})
|