Skip to content

Commit cb65013

Browse files
authored
fix(executor): fix. convergent error edges (#3015)
1 parent 9dbf56f commit cb65013

File tree

2 files changed

+198
-1
lines changed

2 files changed

+198
-1
lines changed

apps/sim/executor/execution/edge-manager.test.ts

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -773,6 +773,176 @@ describe('EdgeManager', () => {
773773
})
774774
})
775775

776+
describe('Multiple error ports to same target', () => {
777+
it('should mark target ready when one source errors and another succeeds', () => {
778+
// This tests the case where a node has multiple incoming error edges
779+
// from different sources. When one source errors (activating its error edge)
780+
// and another source succeeds (deactivating its error edge), the target
781+
// should become ready after both sources complete.
782+
//
783+
// Workflow 1 (errors) ─── error ───┐
784+
// ├──→ Error Handler
785+
// Workflow 7 (succeeds) ─ error ───┘
786+
787+
const workflow1Id = 'workflow-1'
788+
const workflow7Id = 'workflow-7'
789+
const errorHandlerId = 'error-handler'
790+
791+
const workflow1Node = createMockNode(workflow1Id, [
792+
{ target: errorHandlerId, sourceHandle: 'error' },
793+
])
794+
795+
const workflow7Node = createMockNode(workflow7Id, [
796+
{ target: errorHandlerId, sourceHandle: 'error' },
797+
])
798+
799+
const errorHandlerNode = createMockNode(errorHandlerId, [], [workflow1Id, workflow7Id])
800+
801+
const nodes = new Map<string, DAGNode>([
802+
[workflow1Id, workflow1Node],
803+
[workflow7Id, workflow7Node],
804+
[errorHandlerId, errorHandlerNode],
805+
])
806+
807+
const dag = createMockDAG(nodes)
808+
const edgeManager = new EdgeManager(dag)
809+
810+
// Workflow 1 errors first - error edge activates
811+
const readyAfterWorkflow1 = edgeManager.processOutgoingEdges(workflow1Node, {
812+
error: 'Something went wrong',
813+
})
814+
// Error handler should NOT be ready yet (waiting for workflow 7)
815+
expect(readyAfterWorkflow1).not.toContain(errorHandlerId)
816+
817+
// Workflow 7 succeeds - error edge deactivates
818+
const readyAfterWorkflow7 = edgeManager.processOutgoingEdges(workflow7Node, {
819+
result: 'success',
820+
})
821+
// Error handler SHOULD be ready now (workflow 1's error edge activated)
822+
expect(readyAfterWorkflow7).toContain(errorHandlerId)
823+
})
824+
825+
it('should mark target ready when first source succeeds then second errors', () => {
826+
// Opposite order: first source succeeds, then second errors
827+
828+
const workflow1Id = 'workflow-1'
829+
const workflow7Id = 'workflow-7'
830+
const errorHandlerId = 'error-handler'
831+
832+
const workflow1Node = createMockNode(workflow1Id, [
833+
{ target: errorHandlerId, sourceHandle: 'error' },
834+
])
835+
836+
const workflow7Node = createMockNode(workflow7Id, [
837+
{ target: errorHandlerId, sourceHandle: 'error' },
838+
])
839+
840+
const errorHandlerNode = createMockNode(errorHandlerId, [], [workflow1Id, workflow7Id])
841+
842+
const nodes = new Map<string, DAGNode>([
843+
[workflow1Id, workflow1Node],
844+
[workflow7Id, workflow7Node],
845+
[errorHandlerId, errorHandlerNode],
846+
])
847+
848+
const dag = createMockDAG(nodes)
849+
const edgeManager = new EdgeManager(dag)
850+
851+
// Workflow 1 succeeds first - error edge deactivates
852+
const readyAfterWorkflow1 = edgeManager.processOutgoingEdges(workflow1Node, {
853+
result: 'success',
854+
})
855+
// Error handler should NOT be ready yet (waiting for workflow 7)
856+
expect(readyAfterWorkflow1).not.toContain(errorHandlerId)
857+
858+
// Workflow 7 errors - error edge activates
859+
const readyAfterWorkflow7 = edgeManager.processOutgoingEdges(workflow7Node, {
860+
error: 'Something went wrong',
861+
})
862+
// Error handler SHOULD be ready now (workflow 7's error edge activated)
863+
expect(readyAfterWorkflow7).toContain(errorHandlerId)
864+
})
865+
866+
it('should NOT mark target ready when all sources succeed (no errors)', () => {
867+
// When neither source errors, the error handler should NOT run
868+
869+
const workflow1Id = 'workflow-1'
870+
const workflow7Id = 'workflow-7'
871+
const errorHandlerId = 'error-handler'
872+
873+
const workflow1Node = createMockNode(workflow1Id, [
874+
{ target: errorHandlerId, sourceHandle: 'error' },
875+
])
876+
877+
const workflow7Node = createMockNode(workflow7Id, [
878+
{ target: errorHandlerId, sourceHandle: 'error' },
879+
])
880+
881+
const errorHandlerNode = createMockNode(errorHandlerId, [], [workflow1Id, workflow7Id])
882+
883+
const nodes = new Map<string, DAGNode>([
884+
[workflow1Id, workflow1Node],
885+
[workflow7Id, workflow7Node],
886+
[errorHandlerId, errorHandlerNode],
887+
])
888+
889+
const dag = createMockDAG(nodes)
890+
const edgeManager = new EdgeManager(dag)
891+
892+
// Both workflows succeed - both error edges deactivate
893+
const readyAfterWorkflow1 = edgeManager.processOutgoingEdges(workflow1Node, {
894+
result: 'success',
895+
})
896+
expect(readyAfterWorkflow1).not.toContain(errorHandlerId)
897+
898+
const readyAfterWorkflow7 = edgeManager.processOutgoingEdges(workflow7Node, {
899+
result: 'success',
900+
})
901+
// Error handler should NOT be ready (no errors occurred)
902+
expect(readyAfterWorkflow7).not.toContain(errorHandlerId)
903+
})
904+
905+
it('should mark target ready when both sources error', () => {
906+
// When both sources error, the error handler should run
907+
908+
const workflow1Id = 'workflow-1'
909+
const workflow7Id = 'workflow-7'
910+
const errorHandlerId = 'error-handler'
911+
912+
const workflow1Node = createMockNode(workflow1Id, [
913+
{ target: errorHandlerId, sourceHandle: 'error' },
914+
])
915+
916+
const workflow7Node = createMockNode(workflow7Id, [
917+
{ target: errorHandlerId, sourceHandle: 'error' },
918+
])
919+
920+
const errorHandlerNode = createMockNode(errorHandlerId, [], [workflow1Id, workflow7Id])
921+
922+
const nodes = new Map<string, DAGNode>([
923+
[workflow1Id, workflow1Node],
924+
[workflow7Id, workflow7Node],
925+
[errorHandlerId, errorHandlerNode],
926+
])
927+
928+
const dag = createMockDAG(nodes)
929+
const edgeManager = new EdgeManager(dag)
930+
931+
// Workflow 1 errors
932+
const readyAfterWorkflow1 = edgeManager.processOutgoingEdges(workflow1Node, {
933+
error: 'Error 1',
934+
})
935+
expect(readyAfterWorkflow1).not.toContain(errorHandlerId)
936+
937+
// Workflow 7 errors
938+
const readyAfterWorkflow7 = edgeManager.processOutgoingEdges(workflow7Node, {
939+
error: 'Error 2',
940+
})
941+
// Error handler SHOULD be ready (both edges activated)
942+
expect(readyAfterWorkflow7).toContain(errorHandlerId)
943+
})
944+
})
945+
776946
describe('Chained conditions', () => {
777947
it('should handle sequential conditions (condition1 → condition2)', () => {
778948
const condition1Id = 'condition-1'

apps/sim/executor/execution/edge-manager.ts

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const logger = createLogger('EdgeManager')
88

99
export class EdgeManager {
1010
private deactivatedEdges = new Set<string>()
11+
private nodesWithActivatedEdge = new Set<string>()
1112

1213
constructor(private dag: DAG) {}
1314

@@ -35,6 +36,11 @@ export class EdgeManager {
3536
activatedTargets.push(edge.target)
3637
}
3738

39+
// Track nodes that have received at least one activated edge
40+
for (const targetId of activatedTargets) {
41+
this.nodesWithActivatedEdge.add(targetId)
42+
}
43+
3844
const cascadeTargets = new Set<string>()
3945
for (const { target, handle } of edgesToDeactivate) {
4046
this.deactivateEdgeAndDescendants(node.id, target, handle, cascadeTargets)
@@ -71,6 +77,18 @@ export class EdgeManager {
7177
}
7278
}
7379

80+
// Check if any deactivation targets that previously received an activated edge are now ready
81+
for (const { target } of edgesToDeactivate) {
82+
if (
83+
!readyNodes.includes(target) &&
84+
!activatedTargets.includes(target) &&
85+
this.nodesWithActivatedEdge.has(target) &&
86+
this.isTargetReady(target)
87+
) {
88+
readyNodes.push(target)
89+
}
90+
}
91+
7492
return readyNodes
7593
}
7694

@@ -90,6 +108,7 @@ export class EdgeManager {
90108

91109
clearDeactivatedEdges(): void {
92110
this.deactivatedEdges.clear()
111+
this.nodesWithActivatedEdge.clear()
93112
}
94113

95114
/**
@@ -108,6 +127,10 @@ export class EdgeManager {
108127
for (const edgeKey of edgesToRemove) {
109128
this.deactivatedEdges.delete(edgeKey)
110129
}
130+
// Also clear activated edge tracking for these nodes
131+
for (const nodeId of nodeIds) {
132+
this.nodesWithActivatedEdge.delete(nodeId)
133+
}
111134
}
112135

113136
private isTargetReady(targetId: string): boolean {
@@ -210,7 +233,11 @@ export class EdgeManager {
210233
cascadeTargets?.add(targetId)
211234
}
212235

213-
if (this.hasActiveIncomingEdges(targetNode, edgeKey)) {
236+
// Don't cascade if node has active incoming edges OR has received an activated edge
237+
if (
238+
this.hasActiveIncomingEdges(targetNode, edgeKey) ||
239+
this.nodesWithActivatedEdge.has(targetId)
240+
) {
214241
return
215242
}
216243

0 commit comments

Comments
 (0)