mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-05 21:20:12 +00:00
On failed subgraph resolution, return undefined (#6460)
Resolving an executionId to a locatorId can fail if the current workflow does not match the queued one. Rather than throwing an error, the resolution functions have been changed to return undefined. Of note, all consumers of these functions already had checks to ensure the returned value is not undefined. - Even the test for failure state already specified that it should return instead of throwing an error. This PR cleans up a frequent sentry error. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-6460-On-failed-subgraph-resolution-return-undefined-29c6d73d365081c5860ed7d24a99414c) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -61,7 +61,7 @@ function getSubgraphsFromInstanceIds(
|
||||
currentGraph: LGraph | Subgraph,
|
||||
subgraphNodeIds: string[],
|
||||
subgraphs: Subgraph[] = []
|
||||
): Subgraph[] {
|
||||
): Subgraph[] | undefined {
|
||||
// Last segment is the node portion; nothing to do.
|
||||
if (subgraphNodeIds.length === 1) return subgraphs
|
||||
|
||||
@@ -69,7 +69,10 @@ function getSubgraphsFromInstanceIds(
|
||||
if (currentPart === undefined) return subgraphs
|
||||
|
||||
const subgraph = subgraphNodeIdToSubgraph(currentPart, currentGraph)
|
||||
if (!subgraph) throw new Error(`Subgraph not found: ${currentPart}`)
|
||||
if (!subgraph) {
|
||||
console.warn(`Subgraph not found: ${currentPart}`)
|
||||
return undefined
|
||||
}
|
||||
|
||||
subgraphs.push(subgraph)
|
||||
return getSubgraphsFromInstanceIds(subgraph, subgraphNodeIds, subgraphs)
|
||||
@@ -80,7 +83,9 @@ function getSubgraphsFromInstanceIds(
|
||||
* @param nodeId The node ID from execution context (could be execution ID)
|
||||
* @returns The NodeLocatorId
|
||||
*/
|
||||
function executionIdToNodeLocatorId(nodeId: string | number): NodeLocatorId {
|
||||
function executionIdToNodeLocatorId(
|
||||
nodeId: string | number
|
||||
): NodeLocatorId | undefined {
|
||||
const nodeIdStr = String(nodeId)
|
||||
|
||||
if (!nodeIdStr.includes(':')) {
|
||||
@@ -92,6 +97,7 @@ function executionIdToNodeLocatorId(nodeId: string | number): NodeLocatorId {
|
||||
const parts = nodeIdStr.split(':')
|
||||
const localNodeId = parts[parts.length - 1]
|
||||
const subgraphs = getSubgraphsFromInstanceIds(app.graph, parts)
|
||||
if (!subgraphs) return undefined
|
||||
const nodeLocatorId = createNodeLocatorId(subgraphs.at(-1)!.id, localNodeId)
|
||||
return nodeLocatorId
|
||||
}
|
||||
|
||||
@@ -156,14 +156,11 @@ describe('useExecutionStore - NodeLocatorId conversions', () => {
|
||||
expect(result).toBe('123')
|
||||
})
|
||||
|
||||
it('should return null when conversion fails', () => {
|
||||
it('should return undefined when conversion fails', () => {
|
||||
// Mock app.graph.getNodeById to return null (node not found)
|
||||
vi.mocked(app.graph.getNodeById).mockReturnValue(null)
|
||||
|
||||
// This should throw an error as the node is not found
|
||||
expect(() => store.executionIdToNodeLocatorId('999:456')).toThrow(
|
||||
'Subgraph not found: 999'
|
||||
)
|
||||
expect(store.executionIdToNodeLocatorId('999:456')).toBe(undefined)
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
Reference in New Issue
Block a user