From 9ddead24d619ec857459e1fc546604343e0f4880 Mon Sep 17 00:00:00 2001 From: AustinMroz Date: Thu, 30 Oct 2025 12:06:52 -0700 Subject: [PATCH] On failed subgraph resolution, return undefined (#6460) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- src/stores/executionStore.ts | 12 +++++++++--- tests-ui/tests/store/executionStore.test.ts | 7 ++----- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/stores/executionStore.ts b/src/stores/executionStore.ts index 9e4842396..aba407abd 100644 --- a/src/stores/executionStore.ts +++ b/src/stores/executionStore.ts @@ -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 } diff --git a/tests-ui/tests/store/executionStore.test.ts b/tests-ui/tests/store/executionStore.test.ts index 8632c4922..5b7c934ab 100644 --- a/tests-ui/tests/store/executionStore.test.ts +++ b/tests-ui/tests/store/executionStore.test.ts @@ -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) }) })