Comfy-Org/ComfyUI_frontend
1
0
Fork 0
mirror of https://github.com/Comfy-Org/ComfyUI_frontend.git synced 2026-05-04 21:22:07 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: error system cleanup — store separation, DDD fix, test improvements (#10302)

Browse Source 
## Summary

Refactors the error system to improve separation of concerns, fix DDD
layer violations, and address code quality issues.

- Extract `missingNodesErrorStore` from `executionErrorStore`, removing
the delegation pattern that coupled missing-node logic into the
execution error store
- Extract `useNodeErrorFlagSync` composable for node error flag
reconciliation (previously inlined)
- Extract `useErrorClearingHooks` composable with explicit callback
cleanup on node removal
- Extract `useErrorActions` composable to deduplicate telemetry+command
patterns across error card components
- Move `getCnrIdFromNode`/`getCnrIdFromProperties` to
`platform/nodeReplacement` layer (DDD fix)
- Move `missingNodesErrorStore` to `platform/nodeReplacement` (DDD
alignment)
- Add unmount cancellation guard to `useErrorReport` async `onMounted`
- Return watch stop handle from `useNodeErrorFlagSync`
- Add `asyncResolvedIds` eviction on `missingNodesError` reset
- Add `console.warn` to silent catch blocks and empty array guard
- Hoist `useCommandStore` to setup scope, fix floating promises
- Add `data-testid` to error groups, image/video error spans, copy
button
- Update E2E tests to use scoped locators and testids
- Add unit tests for `onNodeRemoved` restoration and double-install
guard

Fixes #9875, Fixes #10027, Fixes #10033, Fixes #10085

## Test plan

- [x] Existing unit tests pass with updated imports and mocks
- [x] New unit tests for `useErrorClearingHooks` (callback restoration,
double-install guard)
- [x] E2E tests updated to use scoped locators and `data-testid`
- [ ] Manual: verify error tab shows runtime errors and missing nodes
correctly
- [ ] Manual: verify "Find on GitHub", "Copy", and "Get Help" buttons
work in error cards

┆Issue is synchronized with this [Notion
page](https://www.notion.so/PR-10302-refactor-error-system-cleanup-store-separation-DDD-fix-test-improvements-3286d73d365081838279d045b8dd957a)
by [Unito](https://www.unito.io)

---------

Co-authored-by: GitHub Action <action@github.com>
This commit is contained in:
jaeone94
2026-03-24 16:43:22 +09:00
committed by GitHub
parent 32a53eeaee
commit 66daa6d645
36 changed files with 868 additions and 630 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
src/platform/nodeReplacement/missingNodeScan.test.ts
View File

@@ -20,8 +20,8 @@ vi.mock('@/utils/graphTraversalUtil', () => ({
getExecutionIdByNode: vi.fn()
}))
vi.mock('@/workbench/extensions/manager/utils/missingNodeErrorUtil', () => ({
getCnrIdFromNode: vi.fn(() => null)
vi.mock('@/platform/nodeReplacement/cnrIdUtil', () => ({
getCnrIdFromNode: vi.fn(() => undefined)
}))
vi.mock('@/i18n', () => ({
@@ -48,11 +48,10 @@ import {
collectAllNodes,
getExecutionIdByNode
} from '@/utils/graphTraversalUtil'
// eslint-disable-next-line import-x/no-restricted-paths
import { getCnrIdFromNode } from '@/workbench/extensions/manager/utils/missingNodeErrorUtil'
import { getCnrIdFromNode } from '@/platform/nodeReplacement/cnrIdUtil'
import { useNodeReplacementStore } from '@/platform/nodeReplacement/nodeReplacementStore'
import { rescanAndSurfaceMissingNodes } from './missingNodeScan'
import { useExecutionErrorStore } from '@/stores/executionErrorStore'
import { useMissingNodesErrorStore } from '@/platform/nodeReplacement/missingNodesErrorStore'
function mockNode(
id: number,
@@ -72,7 +71,7 @@ function mockGraph(): LGraph {
}
function getMissingNodesError(
store: ReturnType<typeof useExecutionErrorStore>
store: ReturnType<typeof useMissingNodesErrorStore>
) {
const error = store.missingNodesError
if (!error) throw new Error('Expected missingNodesError to be defined')
@@ -99,7 +98,7 @@ describe('scanMissingNodes (via rescanAndSurfaceMissingNodes)', () => {
rescanAndSurfaceMissingNodes(mockGraph())
const store = useExecutionErrorStore()
const store = useMissingNodesErrorStore()
expect(store.missingNodesError).toBeNull()
})
@@ -112,7 +111,7 @@ describe('scanMissingNodes (via rescanAndSurfaceMissingNodes)', () => {
rescanAndSurfaceMissingNodes(mockGraph())
const store = useExecutionErrorStore()
const store = useMissingNodesErrorStore()
const error = getMissingNodesError(store)
expect(error.nodeTypes).toHaveLength(2)
})
@@ -129,7 +128,7 @@ describe('scanMissingNodes (via rescanAndSurfaceMissingNodes)', () => {
rescanAndSurfaceMissingNodes(mockGraph())
const store = useExecutionErrorStore()
const store = useMissingNodesErrorStore()
const error = getMissingNodesError(store)
expect(error.nodeTypes).toHaveLength(1)
const missing = error.nodeTypes[0]
@@ -142,7 +141,7 @@ describe('scanMissingNodes (via rescanAndSurfaceMissingNodes)', () => {
rescanAndSurfaceMissingNodes(mockGraph())
const store = useExecutionErrorStore()
const store = useMissingNodesErrorStore()
const error = getMissingNodesError(store)
const missing = error.nodeTypes[0]
expect(typeof missing !== 'string' && missing.nodeId).toBe('exec-42')
@@ -154,7 +153,7 @@ describe('scanMissingNodes (via rescanAndSurfaceMissingNodes)', () => {
rescanAndSurfaceMissingNodes(mockGraph())
const store = useExecutionErrorStore()
const store = useMissingNodesErrorStore()
const error = getMissingNodesError(store)
const missing = error.nodeTypes[0]
expect(typeof missing !== 'string' && missing.nodeId).toBe('99')
@@ -167,7 +166,7 @@ describe('scanMissingNodes (via rescanAndSurfaceMissingNodes)', () => {
rescanAndSurfaceMissingNodes(mockGraph())
const store = useExecutionErrorStore()
const store = useMissingNodesErrorStore()
const error = getMissingNodesError(store)
const missing = error.nodeTypes[0]
expect(typeof missing !== 'string' && missing.cnrId).toBe(
@@ -194,7 +193,7 @@ describe('scanMissingNodes (via rescanAndSurfaceMissingNodes)', () => {
rescanAndSurfaceMissingNodes(mockGraph())
const store = useExecutionErrorStore()
const store = useMissingNodesErrorStore()
const error = getMissingNodesError(store)
const missing = error.nodeTypes[0]
expect(typeof missing !== 'string' && missing.isReplaceable).toBe(true)
@@ -209,7 +208,7 @@ describe('scanMissingNodes (via rescanAndSurfaceMissingNodes)', () => {
rescanAndSurfaceMissingNodes(mockGraph())
const store = useExecutionErrorStore()
const store = useMissingNodesErrorStore()
const error = getMissingNodesError(store)
const missing = error.nodeTypes[0]
expect(typeof missing !== 'string' && missing.isReplaceable).toBe(false)
@@ -225,7 +224,7 @@ describe('scanMissingNodes (via rescanAndSurfaceMissingNodes)', () => {
rescanAndSurfaceMissingNodes(mockGraph())
const store = useExecutionErrorStore()
const store = useMissingNodesErrorStore()
const error = getMissingNodesError(store)
const missing = error.nodeTypes[0]
expect(typeof missing !== 'string' && missing.type).toBe('OriginalType')