Merge pull request #504 from hjotha/submit/error-handler-rejoin-if-else

fix: rejoin empty custom error handler at IF ELSE first activity

Author: ako

mdl/executor/cmd_microflows_builder_control.go

@@ -41,6 +41,36 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
 	// a branch's RETURN affecting the parent flow state prematurely
 	savedEndsWithReturn := fb.endsWithReturn
 
+	// Capture an incoming empty custom error handler whose source is the
+	// previous call activity (set by finishCustomErrorHandler via
+	// registerEmptyCustomErrorHandlerWithSkip) when the call has a
+	// non-empty output variable (skipVar). The Studio Pro authored pattern
+	// wires the error edge into the IF's ELSE branch — at the first
+	// activity in the ELSE body — so the fallback path handles both "no
+	// value" and "error" cases. Left un-rejoined,
+	// terminatePendingErrorHandlersAtEnd would synthesise a phantom
+	// EndEvent; naively rejoining at the split would bypass the output
+	// variable declaration and trigger CE0108 "variable not in scope" on
+	// subsequent activities.
+	//
+	// We consume the pending state here so the rejoin is emitted in the
+	// `lastElseID == ""` block below (when the first ELSE activity is
+	// created); the corresponding builder fields are cleared so downstream
+	// logic does not see the handler twice.
+	pendingElseErrorOrigin := model.ID("")
+	if fb.errorHandlerSource != "" && fb.errorHandlerTailFrom == fb.errorHandlerSource &&
+		fb.errorHandlerSkipVar != "" && fb.errorHandlerTailIsSource &&
+		hasElseBody && len(s.ElseBody) > 0 &&
+		bothReturn && // only when both branches terminate — a continuation in THEN would need a different rejoin target
+		statementReferencesVar(s, fb.errorHandlerSkipVar) &&
+		firstElseIsLeafActivity(s.ElseBody) {
+		pendingElseErrorOrigin = fb.errorHandlerSource
+		fb.errorHandlerSource = ""
+		fb.errorHandlerTailFrom = ""
+		fb.errorHandlerSkipVar = ""
+		fb.errorHandlerTailIsSource = false
+	}
+
 	// Save current center line position
 	splitX := fb.posX
 	centerY := fb.posY // This is the center line for the happy path
@@ -223,6 +253,17 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
 					flow := newDownwardFlowWithCase(splitID, actID, "false")
 					applyUserAnchors(flow, falseBranchAnchor, branchDestinationAnchor(falseBranchAnchor, thisAnchor))
 					fb.flows = append(fb.flows, flow)
+					// Rejoin a pending empty custom error handler from the
+					// previous call activity at the first ELSE activity.
+					// Mendix authors the error edge directly into the ELSE
+					// fallback path (same entry point as the split `false`
+					// branch), preserving output-variable scope while giving
+					// the error case the same handling as the happy-path
+					// fallback.
+					if pendingElseErrorOrigin != "" {
+						fb.addEmptyErrorHandlerRejoinFlowFrom(splitID, pendingElseErrorOrigin, actID)
+						pendingElseErrorOrigin = ""
+					}
 				} else {
 					var flow *microflows.SequenceFlow
 					originAnchor := prevElseAnchor
@@ -463,6 +504,22 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
 	return splitID
 }
 
+// firstElseIsLeafActivity reports whether the ELSE body's first statement is
+// a plain activity (not a compound split/if/loop). Rejoining an error flow
+// onto a compound statement's internal split/merge would need extra plumbing
+// that the caller is not equipped for; fall back to the default phantom-
+// EndEvent terminator in that case.
+func firstElseIsLeafActivity(stmts []ast.MicroflowStatement) bool {
+	if len(stmts) == 0 {
+		return false
+	}
+	switch stmts[0].(type) {
+	case *ast.IfStmt, *ast.LoopStmt, *ast.WhileStmt, *ast.EnumSplitStmt:
+		return false
+	}
+	return true
+}
+
 func bodyHasContinuingCustomErrorHandler(stmts []ast.MicroflowStatement) bool {
 	for _, stmt := range stmts {
 		if eh := statementErrorHandling(stmt); eh != nil {

mdl/executor/error_handler_if_else_rejoin_test.go

@@ -0,0 +1,111 @@
+package executor
+
+import (
+	"testing"
+
+	"github.com/mendixlabs/mxcli/mdl/ast"
+	"github.com/mendixlabs/mxcli/model"
+	"github.com/mendixlabs/mxcli/sdk/microflows"
+)
+
+// Empty custom error handler followed by an IF that checks the call's
+// output variable: the error edge must rejoin the fallback path at the
+// first ELSE activity, sharing a merge with the split's `false` branch.
+// Without this rejoin, terminatePendingErrorHandlersAtEnd synthesises a
+// phantom EndEvent at the microflow tail (CE0079); wiring the error edge
+// straight to the IF split bypasses the output-variable declaration and
+// triggers CE0108 "variable not in scope" downstream.
+func TestEmptyErrorHandlerRejoinsAtIfElseFirstActivity(t *testing.T) {
+	fb := &flowBuilder{
+		spacing:      HorizontalSpacing,
+		measurer:     &layoutMeasurer{},
+		declaredVars: map[string]string{"R": "String"},
+	}
+
+	oc := fb.buildFlowGraph([]ast.MicroflowStatement{
+		&ast.CallMicroflowStmt{
+			MicroflowName:  ast.QualifiedName{Module: "M", Name: "Helper"},
+			OutputVariable: "R",
+			ErrorHandling: &ast.ErrorHandlingClause{
+				Type: ast.ErrorHandlingCustomWithoutRollback,
+				Body: nil,
+			},
+		},
+		&ast.IfStmt{
+			Condition: &ast.VariableExpr{Name: "R"},
+			ThenBody: []ast.MicroflowStatement{
+				&ast.LogStmt{Level: ast.LogInfo, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "ok"}},
+				&ast.ReturnStmt{},
+			},
+			ElseBody: []ast.MicroflowStatement{
+				&ast.LogStmt{Level: ast.LogError, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "fail"}},
+				&ast.ReturnStmt{},
+			},
+		},
+	}, nil)
+
+	// Count EndEvents — should be 2 (one per IF branch), no phantom at mf end.
+	endEvents := 0
+	for _, obj := range oc.Objects {
+		if _, ok := obj.(*microflows.EndEvent); ok {
+			endEvents++
+		}
+	}
+	if endEvents != 2 {
+		t.Errorf("got %d EndEvents, want 2", endEvents)
+	}
+
+	// Find call activity, if split, first-else activity
+	var callID, splitID, firstElseID model.ID
+	logCount := 0
+	for _, obj := range oc.Objects {
+		switch o := obj.(type) {
+		case *microflows.ActionActivity:
+			if _, ok := o.Action.(*microflows.MicroflowCallAction); ok {
+				callID = o.ID
+			}
+			if _, ok := o.Action.(*microflows.LogMessageAction); ok {
+				logCount++
+				if logCount == 2 {
+					// Second log in visitation order = the else body log ("fail")
+					firstElseID = o.ID
+				}
+			}
+		case *microflows.ExclusiveSplit:
+			splitID = o.ID
+		}
+	}
+	if callID == "" || splitID == "" || firstElseID == "" {
+		t.Fatal("missing call/split/firstElse")
+	}
+
+	// Error flow must originate at call and terminate at a merge reachable from split(false).
+	// Trace error flow from call:
+	var errorDest model.ID
+	for _, f := range oc.Flows {
+		if f.OriginID == callID && f.IsErrorHandler {
+			errorDest = f.DestinationID
+			break
+		}
+	}
+	if errorDest == "" {
+		t.Fatal("no error-handler flow from call")
+	}
+	// errorDest should be a merge that also receives the split false branch and leads to firstElseID
+	mergeReachesFirstElse := false
+	splitFalseReachesMerge := false
+	for _, f := range oc.Flows {
+		if f.OriginID == errorDest && f.DestinationID == firstElseID {
+			mergeReachesFirstElse = true
+		}
+		if f.OriginID == splitID && f.DestinationID == errorDest {
+			splitFalseReachesMerge = true
+		}
+	}
+	if !mergeReachesFirstElse {
+		t.Errorf("merge %s does not flow to first-else activity %s", string(errorDest)[:6], string(firstElseID)[:6])
+	}
+	if !splitFalseReachesMerge {
+		t.Errorf("split %s (false) does not flow to merge %s", string(splitID)[:6], string(errorDest)[:6])
+	}
+}