Skip to content

Commit 1eeef12

Browse files
authored
Merge pull request #507 from hjotha/submit/rest-retry-loop-error-handler
fix: loop-back retry error handler to merge before source activity (#506)
2 parents 4270259 + 25fde3d commit 1eeef12

4 files changed

Lines changed: 333 additions & 3 deletions

File tree

mdl/executor/cmd_microflows_builder.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,13 @@ type flowBuilder struct {
3232
errors []string // Validation errors collected during build
3333
measurer *layoutMeasurer // For measuring statement dimensions
3434
nextConnectionPoint model.ID // For compound statements: the exit point differs from entry point
35+
// incomingRedirect, when set, instructs the next enclosing flow emission to
36+
// terminate at this merge/activity ID instead of the most recently emitted
37+
// activity. Used by retry-loop error handlers (where a merge is inserted
38+
// before the source activity so the normal inbound flow enters the merge
39+
// and the error-handler tail loops back to the same merge). Cleared after
40+
// the outer loop consumes it for the next flow.
41+
incomingRedirect model.ID
3542
nextFlowCase string // If set, next connecting flow uses this case value (for merge-less splits)
3643
// nextFlowAnchor carries the branch-specific FlowAnchors that should be
3744
// applied to the flow created by the NEXT iteration of buildFlowGraph.

mdl/executor/cmd_microflows_builder_flows.go

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,115 @@ func (fb *flowBuilder) finishCustomErrorHandler(activityID model.ID, activityX i
5151
return
5252
}
5353
if len(eh.Body) > 0 {
54+
// Retry-loop pattern: error body ends with an IF whose non-terminating
55+
// branch should loop back to the source activity (Studio Pro authors
56+
// this as a merge placed before the source, with the handler tail
57+
// returning to that merge). MDL cannot express the loop-back directly,
58+
// so we detect the shape and wire the topology ourselves.
59+
if isRetryLoopErrorHandler(eh.Body) {
60+
fb.buildRetryLoopErrorHandler(activityID, activityX, eh.Body)
61+
return
62+
}
5463
mergeID := fb.addErrorHandlerFlow(activityID, activityX, eh.Body)
5564
fb.handleErrorHandlerMergeWithSkip(mergeID, activityID, outputVar)
5665
return
5766
}
5867
fb.registerEmptyCustomErrorHandlerWithSkip(activityID, eh, outputVar)
5968
}
6069

70+
// isRetryLoopErrorHandler reports whether the error-handler body looks like
71+
// a retry loop: the last statement is an IF whose else branch terminates
72+
// (via RAISE ERROR or RETURN) and whose then branch continues. That shape
73+
// mirrors the Studio Pro retry pattern where the non-terminating branch
74+
// loops back to the source activity to re-attempt it.
75+
func isRetryLoopErrorHandler(body []ast.MicroflowStatement) bool {
76+
if len(body) == 0 {
77+
return false
78+
}
79+
ifStmt, ok := body[len(body)-1].(*ast.IfStmt)
80+
if !ok {
81+
return false
82+
}
83+
if len(ifStmt.ThenBody) == 0 || len(ifStmt.ElseBody) == 0 {
84+
return false
85+
}
86+
thenTerminates := bodyTerminates(ifStmt.ThenBody)
87+
elseTerminates := bodyTerminates(ifStmt.ElseBody)
88+
// Exactly one branch must terminate (the "error" branch) and one must
89+
// continue (the "retry" branch). If both or neither terminate, the shape
90+
// is some other IF and the standard merge-forward path still applies.
91+
return thenTerminates != elseTerminates
92+
}
93+
94+
// bodyTerminates reports whether a statement body ends with a terminator
95+
// (RAISE ERROR, RETURN, or nested IF where every branch terminates).
96+
func bodyTerminates(body []ast.MicroflowStatement) bool {
97+
if len(body) == 0 {
98+
return false
99+
}
100+
last := body[len(body)-1]
101+
switch s := last.(type) {
102+
case *ast.RaiseErrorStmt:
103+
return true
104+
case *ast.ReturnStmt:
105+
return true
106+
case *ast.IfStmt:
107+
if len(s.ElseBody) == 0 {
108+
return false
109+
}
110+
return bodyTerminates(s.ThenBody) && bodyTerminates(s.ElseBody)
111+
}
112+
return false
113+
}
114+
115+
// buildRetryLoopErrorHandler wires a retry-loop topology for an error handler
116+
// whose body ends with a terminating/continuing IF. The non-terminating
117+
// branch's tail loops back to a new merge placed before the source activity;
118+
// the merge then feeds into the source. The outer loop consumes
119+
// fb.incomingRedirect so the normal inbound flow also terminates at the
120+
// merge instead of directly at the source.
121+
func (fb *flowBuilder) buildRetryLoopErrorHandler(sourceActivityID model.ID, sourceX int, errorBody []ast.MicroflowStatement) {
122+
// Build the handler activities with tracking of where the non-terminating
123+
// branch's tail ends up. We reuse addErrorHandlerFlow; it returns the tail
124+
// that would have forwarded to the next main-path activity. For a retry
125+
// loop, that tail is the continue-branch exit of the trailing IF.
126+
tail := fb.addErrorHandlerFlow(sourceActivityID, sourceX, errorBody)
127+
if tail.id == "" {
128+
// The handler terminates unexpectedly — nothing to loop back.
129+
return
130+
}
131+
132+
// Insert a merge just left of the source activity, on the main flow row.
133+
merge := &microflows.ExclusiveMerge{
134+
BaseMicroflowObject: microflows.BaseMicroflowObject{
135+
BaseElement: model.BaseElement{ID: model.ID(types.GenerateID())},
136+
Position: model.Point{X: sourceX - fb.spacing/2, Y: fb.baseY},
137+
Size: model.Size{Width: MergeSize, Height: MergeSize},
138+
},
139+
}
140+
fb.objects = append(fb.objects, merge)
141+
142+
// Merge -> source (normal flow into the REST/microflow call)
143+
fb.flows = append(fb.flows, newHorizontalFlow(merge.ID, sourceActivityID))
144+
145+
// Handler tail -> merge (loop-back). Authored Studio Pro flows mark this
146+
// edge as a normal SequenceFlow (not IsErrorHandler) — the error flow
147+
// marker only applies to the SOURCE → first-handler-activity edge, which
148+
// addErrorHandlerFlow already emitted. Using a plain horizontal flow here
149+
// avoids triggering spurious CE0136/CE0019 diagnostics that fire when a
150+
// retrieve's start variable is deemed to flow through an error edge.
151+
loopFlow := newHorizontalFlow(tail.id, merge.ID)
152+
if tail.caseValue != "" {
153+
applyDeferredFlowCase(loopFlow, tail.caseValue, tail.flowAnchor)
154+
} else if tail.flowAnchor != nil {
155+
applyDeferredFlowCase(loopFlow, "", tail.flowAnchor)
156+
}
157+
fb.flows = append(fb.flows, loopFlow)
158+
159+
// Next inbound normal flow must terminate at the merge, not at the source.
160+
fb.incomingRedirect = merge.ID
161+
}
162+
61163
func (fb *flowBuilder) registerEmptyCustomErrorHandlerWithSkip(activityID model.ID, eh *ast.ErrorHandlingClause, skipVar string) {
62164
if !isEmptyCustomErrorHandler(eh) {
63165
return

mdl/executor/cmd_microflows_builder_graph.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,21 @@ func (fb *flowBuilder) buildFlowGraph(stmts []ast.MicroflowStatement, returns *a
7676
activityID := fb.addStatement(stmt)
7777
if activityID != "" {
7878
fb.applyPendingAnnotations(activityID)
79-
// Connect to previous object with horizontal SequenceFlow
79+
// Connect to previous object with horizontal SequenceFlow.
80+
// When incomingRedirect is set (retry-loop error handler built a
81+
// merge before the activity), the inbound flow terminates at the
82+
// merge instead of the activity itself.
83+
inboundTarget := activityID
84+
if fb.incomingRedirect != "" {
85+
inboundTarget = fb.incomingRedirect
86+
fb.incomingRedirect = ""
87+
}
8088
var flow *microflows.SequenceFlow
8189
if pendingCase != "" {
82-
flow = newHorizontalFlowWithCase(lastID, activityID, pendingCase)
90+
flow = newHorizontalFlowWithCase(lastID, inboundTarget, pendingCase)
8391
pendingCase = ""
8492
} else {
85-
flow = newHorizontalFlow(lastID, activityID)
93+
flow = newHorizontalFlow(lastID, inboundTarget)
8694
}
8795
// Prefer the pendingFlowAnchor (carried from a guard-pattern IF's
8896
// branch) over the previous statement's own anchor — it encodes
Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
package executor
4+
5+
import (
6+
"testing"
7+
8+
"github.com/mendixlabs/mxcli/mdl/ast"
9+
"github.com/mendixlabs/mxcli/model"
10+
"github.com/mendixlabs/mxcli/sdk/microflows"
11+
)
12+
13+
// Retry-loop error handler: a call activity with a custom error handler
14+
// whose body ends with an IF where the ELSE raises and the THEN performs
15+
// retry actions must produce a merge placed before the source activity,
16+
// with the handler tail looping back to the merge. Without this topology
17+
// the merge falls after the source and subsequent activities referencing
18+
// the call's output variable trigger CE0108 "variable not in scope".
19+
func TestRetryLoopErrorHandlerLoopsBackToSource(t *testing.T) {
20+
fb := &flowBuilder{
21+
spacing: HorizontalSpacing,
22+
measurer: &layoutMeasurer{},
23+
declaredVars: map[string]string{"R": "String", "RetryCount": "Integer"},
24+
}
25+
26+
oc := fb.buildFlowGraph([]ast.MicroflowStatement{
27+
&ast.DeclareStmt{
28+
Variable: "RetryCount",
29+
Type: ast.DataType{Kind: ast.TypeInteger},
30+
InitialValue: &ast.LiteralExpr{Kind: ast.LiteralInteger, Value: "0"},
31+
},
32+
&ast.CallMicroflowStmt{
33+
MicroflowName: ast.QualifiedName{Module: "M", Name: "Fetch"},
34+
OutputVariable: "R",
35+
ErrorHandling: &ast.ErrorHandlingClause{
36+
Type: ast.ErrorHandlingCustomWithoutRollback,
37+
Body: []ast.MicroflowStatement{
38+
&ast.LogStmt{Level: ast.LogError, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "fetch failed"}},
39+
&ast.IfStmt{
40+
Condition: &ast.BinaryExpr{
41+
Operator: "<",
42+
Left: &ast.VariableExpr{Name: "RetryCount"},
43+
Right: &ast.LiteralExpr{Kind: ast.LiteralInteger, Value: "3"},
44+
},
45+
ThenBody: []ast.MicroflowStatement{
46+
&ast.MfSetStmt{
47+
Target: "RetryCount",
48+
Value: &ast.BinaryExpr{
49+
Operator: "+",
50+
Left: &ast.VariableExpr{Name: "RetryCount"},
51+
Right: &ast.LiteralExpr{Kind: ast.LiteralInteger, Value: "1"},
52+
},
53+
},
54+
},
55+
ElseBody: []ast.MicroflowStatement{
56+
&ast.RaiseErrorStmt{},
57+
},
58+
},
59+
},
60+
},
61+
},
62+
&ast.LogStmt{Level: ast.LogInfo, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "ok"}},
63+
&ast.ReturnStmt{},
64+
}, nil)
65+
66+
// Find the call activity, the retry-body log/set activities, and the merge.
67+
var callID, logAfterID model.ID
68+
var mergeIDs []model.ID
69+
logSeen := 0
70+
for _, obj := range oc.Objects {
71+
switch o := obj.(type) {
72+
case *microflows.ActionActivity:
73+
if _, ok := o.Action.(*microflows.MicroflowCallAction); ok {
74+
callID = o.ID
75+
}
76+
if _, ok := o.Action.(*microflows.LogMessageAction); ok {
77+
logSeen++
78+
// logSeen==1 is the error-handler log inside the handler body.
79+
// logSeen==2 is the main-path log after the call.
80+
if logSeen == 2 {
81+
logAfterID = o.ID
82+
}
83+
}
84+
case *microflows.ExclusiveMerge:
85+
mergeIDs = append(mergeIDs, o.ID)
86+
}
87+
}
88+
if callID == "" {
89+
t.Fatal("no call activity found")
90+
}
91+
if logAfterID == "" {
92+
t.Fatal("no post-call log activity found")
93+
}
94+
if len(mergeIDs) == 0 {
95+
t.Fatal("no merge node found — retry-loop topology was not built")
96+
}
97+
98+
// The merge must sit on the call's inbound path: there must be an edge
99+
// prev->merge (not prev->call) and an edge merge->call. The handler
100+
// tail loops back to the merge as a plain SequenceFlow (not marked
101+
// IsErrorHandler — the error marker applies only to the source→first
102+
// handler activity edge, which addErrorHandlerFlow emits separately).
103+
var mergeToCallFound bool
104+
var prevToMergeFound bool
105+
var tailToMergeFound bool
106+
// Find the retry-branch tail activity (the ChangeVariable that
107+
// increments the retry counter in the test fixture).
108+
var tailID model.ID
109+
for _, obj := range oc.Objects {
110+
if a, ok := obj.(*microflows.ActionActivity); ok {
111+
if _, isChange := a.Action.(*microflows.ChangeVariableAction); isChange {
112+
tailID = a.ID
113+
}
114+
}
115+
}
116+
for _, f := range oc.Flows {
117+
for _, mID := range mergeIDs {
118+
if f.DestinationID == callID && f.OriginID == mID {
119+
mergeToCallFound = true
120+
}
121+
if f.DestinationID == mID && f.OriginID == tailID && !f.IsErrorHandler {
122+
tailToMergeFound = true
123+
}
124+
if f.DestinationID == mID && !f.IsErrorHandler && f.OriginID != mID && f.OriginID != tailID {
125+
prevToMergeFound = true
126+
}
127+
}
128+
// No flow should terminate directly at the call from a non-merge origin.
129+
if f.DestinationID == callID && !f.IsErrorHandler {
130+
originIsMerge := false
131+
for _, mID := range mergeIDs {
132+
if f.OriginID == mID {
133+
originIsMerge = true
134+
}
135+
}
136+
if !originIsMerge {
137+
t.Errorf("normal inbound flow to call %s came from non-merge origin %s; retry-loop merge was not inserted",
138+
shortID(callID), shortID(f.OriginID))
139+
}
140+
}
141+
}
142+
if !mergeToCallFound {
143+
t.Error("missing merge -> call flow")
144+
}
145+
if !prevToMergeFound {
146+
t.Error("missing prev -> merge normal flow")
147+
}
148+
if !tailToMergeFound {
149+
t.Error("missing handler-tail -> merge (loop-back) flow")
150+
}
151+
}
152+
153+
// Non-retry error handlers (both branches return, neither branch raises,
154+
// or no trailing IF) must still use the forward-merge path.
155+
func TestNonRetryErrorHandlerUsesForwardMerge(t *testing.T) {
156+
fb := &flowBuilder{
157+
spacing: HorizontalSpacing,
158+
measurer: &layoutMeasurer{},
159+
declaredVars: map[string]string{"R": "String"},
160+
}
161+
162+
oc := fb.buildFlowGraph([]ast.MicroflowStatement{
163+
&ast.CallMicroflowStmt{
164+
MicroflowName: ast.QualifiedName{Module: "M", Name: "Fetch"},
165+
OutputVariable: "R",
166+
ErrorHandling: &ast.ErrorHandlingClause{
167+
Type: ast.ErrorHandlingCustomWithoutRollback,
168+
Body: []ast.MicroflowStatement{
169+
// Single log — no trailing IF, no retry shape.
170+
&ast.LogStmt{Level: ast.LogError, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "failed"}},
171+
},
172+
},
173+
},
174+
&ast.LogStmt{Level: ast.LogInfo, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "ok"}},
175+
&ast.ReturnStmt{},
176+
}, nil)
177+
178+
// The call should have an inbound flow from the previous activity (the
179+
// StartEvent or a fb-generated start), NOT via a merge. If a merge were
180+
// incorrectly inserted, every such non-retry microflow would gain an
181+
// extra unnecessary node.
182+
var callID model.ID
183+
for _, obj := range oc.Objects {
184+
if a, ok := obj.(*microflows.ActionActivity); ok {
185+
if _, isCall := a.Action.(*microflows.MicroflowCallAction); isCall {
186+
callID = a.ID
187+
}
188+
}
189+
}
190+
if callID == "" {
191+
t.Fatal("no call activity")
192+
}
193+
for _, f := range oc.Flows {
194+
if f.DestinationID == callID && !f.IsErrorHandler {
195+
// Origin should be a non-merge (StartEvent or a plain activity).
196+
for _, obj := range oc.Objects {
197+
if obj.GetID() == f.OriginID {
198+
if _, isMerge := obj.(*microflows.ExclusiveMerge); isMerge {
199+
t.Errorf("non-retry handler unexpectedly inserted a merge before call")
200+
}
201+
}
202+
}
203+
}
204+
}
205+
}
206+
207+
func shortID(id model.ID) string {
208+
s := string(id)
209+
if len(s) > 8 {
210+
return s[:8]
211+
}
212+
return s
213+
}

0 commit comments

Comments
 (0)