fix(microflows): an error handler inside a branch continues after its failing activity

.claude/skills/fix-issue/findings/mdl-executor/2026-10-22-branch-error-handler-rejoins-outside-branch.json

@@ -0,0 +1 @@
+{"area": "mdl/executor (microflow builder)", "date": "2026-10-22", "symptom": "A fall-through custom error handler (no return / raise error) on an activity inside an IF, CASE or split type branch was wired outside the branch. With a result variable it rejoined on the flow INTO the split, so a failing call re-ran the decision and called again (a loop; DESCRIBE shows `merge rejoin1;` before the `if`). Without one it ended the microflow at an end event of its own (DESCRIBE shows a `return;` the script did not contain). In a branch ending in return it rejoined after the IF. `mx check` reports 0 errors for all of them (mendixlabs/mxcli#1565).", "cause": "The builder keeps the handler pending until the next flow is drawn from the failing activity. buildFlowGraph settles it on every flow it draws, but the branch loops settled only an EMPTY handler (`if fb.emptyErrorHandlerFrom == lastID`), and the IF-without-ELSE and split type loops not even that, so a handler with a body left the branch pending. The parent draws the flow into the split only after building it, and a handler with a result variable settles on the next flow whatever its origin: that flow was the one into the split. Without a result variable nothing matched, and the end-of-flow pass gave it an end event.", "file": "mdl/executor/cmd_microflows_builder_flows.go (enterBranchErrorHandlers / takeBranchErrorHandlers / settleBranchErrorHandlersOn / addBranchErrorHandlerFlowForStatement / queueBranchErrorHandlersAt; addPendingErrorHandlerFlowTo removed), cmd_microflows_builder_control.go (IF: all three branch loops settle on each flow and on the exit flow to the merge; leftovers rejoin on the flow leaving the IF; thenNeedsErrorMerge removed), cmd_microflows_builder_actions.go (CASE and split type branches); test cmd_microflows_builder_branch_handler_test.go; example mdl-examples/bug-tests/1565-branch-error-handler-rejoin.mdl", "insight": "Making the branch loops settle on every flow is not enough on its own. (1) The handlers pending when a split starts belong to the activity BEFORE it, and a result-variable handler settles on any flow, so it would rejoin in the middle of the first branch: each branch sets the outer handlers aside and restores them. (2) In a branch that ends in return, a result-variable handler is deliberately sent to the code after the split, the shared error path of a success tail built on the result (TestBuildFlowGraph_OutputHandlerInReturningBranchSkipsDerivedSuccessTail); left pending, it would settle on the next flow the parent draws, the one INTO the split. Such leftovers are pinned to the split exit (merge, continuing branch, or the split itself when every branch returns) by making that exit their source and clearing the skip variable, so they settle on the flow leaving the split. Assert the graph (where the handler tail reaches through merges, end-event count, no cycle), not mx check, which accepts every broken shape.", "refs": ["mendixlabs/mxcli#1565"], "ce": []}

CHANGELOG.md

@@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Fixed
 
+- **An error handler inside a branch continues after its failing activity** (mendixlabs/mxcli#1565) — a custom error handler that does not end in `return` or `raise error`, on an activity inside an IF, CASE or split type branch, was wired outside the branch. With a result variable (`$N = call microflow … on error without rollback begin … end error;`) it rejoined on the flow into the split, so a failing call re-ran the decision and called again, for as long as it kept failing; without one it ended the microflow at an end event of its own, skipping the rest of the branch and everything after it; in a branch ending in `return` it skipped to the code after the IF. `mx check` accepts all three. Each branch now rejoins its own handlers right after the failing activity, as the top level does, or on the branch's way out when the activity is its last. In a branch that ends in `return`, a handler on an activity with a result still goes to the code after the split, the branch's shared error path, but on the flow leaving the split rather than the one into it. A handler on an activity before the split is unchanged. The third case is a behaviour change for a handler on an activity without a result: it now runs the rest of its branch, `return` included, rather than the code after the IF.
 - **A specialization of a non-persistent entity reads as non-persistent** (mendixlabs/mxcli#1600) — `describe entity Probe.Child` printed `create or modify persistent entity Probe.Child extends Probe.Base (` when `Probe.Base` is non-persistent, so a reader took the child for a table and the output stated a combination Mendix does not have. A specialization stores no persistable flag of its own; it takes its root generalization's, and the reader defaulted it to persistent. The flag is now resolved along the generalization chain, across modules and into the System module (whose `HttpRequest`, `HttpResponse` and `SoapFault` read persistent for the same reason), and the fix reaches everything that reads it: `describe`, the catalog's `EntityType`, the domain-model diagrams, and `alter entities … where persistent`, which used to add its attribute to such a child as well. Re-running the describe output leaves the entity unchanged.
 - **`go mod tidy` runs again, and the expression round-trip test runs for the first time** — `mdl/exprcheck/roundtrip_test.go` imported `mdl/backend/mpr` (the legacy engine, deleted) plus `executor.GetHierarchyForMining` and `executor.DescribeMicroflowToString` (both removed). It never broke a build because it sat behind `//go:build roundtrip`, a tag no Makefile target and no CI job has ever passed — the only file in the tree using it. `go mod tidy` considers every build tag, so it was the one command that still saw the file, and it failed with "no matching versions for query latest" on the deleted package, which reads as a dependency problem rather than a dead test. The test is ported to `mdl/executor/exprcheck_roundtrip_test.go` under the `integration` tag, where `setupTestEnv`/`describeMDL` already provide the real-project harness and `make test-integration-executor` actually runs it; the removed hierarchy helper is replaced by building the container-to-module map from `backend.ListModules()`. Its assertion is worth keeping: describing every microflow and re-parsing each expression with the robust parser catches grammar gaps — a missing `parsePrimary` branch, a missing `funcTable` entry, a missing slot constraint — that the unit tests cannot, since those only feed the parser expressions someone thought to write down. It now also fails rather than passing silently when the fixture project yields no microflows or no expression slots.
 

mdl-examples/bug-tests/1565-branch-error-handler-rejoin.mdl

@@ -0,0 +1,92 @@
+mdl 1;
+-- ============================================================================
+-- mendixlabs/mxcli#1565: a fall-through error handler inside a branch
+-- ============================================================================
+--
+-- A custom error handler that does not end in `return` or `raise error`
+-- continues with the statement after the failing activity. Inside a branch
+-- (IF, CASE, split type) it was left pending past the branch:
+--
+--   * with a result variable it rejoined on the flow INTO the split, so a
+--     failing call re-ran the decision and called again: a loop for as long
+--     as the call kept failing (DESCRIBE: `merge rejoin1;` before the `if`);
+--   * without one it ended the microflow at an end event of its own, so the
+--     rest of the branch and everything after it never ran (DESCRIBE: a
+--     `return;` in the handler that the script did not contain);
+--   * in a branch ending in `return`, it rejoined after the IF instead.
+--
+-- `mx check` reports 0 errors for all three, so only the flow shows it. After
+-- the fix, DESCRIBE prints each handler without `return;` or `join`, with the
+-- rejoin merge folded onto the failing call as `@merge(x, y)`, and re-running
+-- the DESCRIBE output changes nothing.
+
+create module BugTest1565;
+
+create enumeration BugTest1565.Color (Red 'Red', Green 'Green');
+
+create microflow BugTest1565.Sub () returns Integer
+begin
+  return 1;
+end;
+
+create microflow BugTest1565.SubNoResult ()
+begin
+  log info node 'BugTest1565' 'sub';
+end;
+
+-- Rejoins at 'after call'; no loop back to the split.
+create microflow BugTest1565.WithResult ($Flag: Boolean)
+begin
+  log info node 'BugTest1565' 'before';
+  if $Flag then
+    $N = call microflow BugTest1565.Sub() on error without rollback begin
+      log warning node 'BugTest1565' 'failed';
+    end error;
+    log info node 'BugTest1565' 'after call';
+  end if;
+  log info node 'BugTest1565' 'end';
+end;
+
+-- Rejoins at 'after call'; one end event.
+create microflow BugTest1565.WithoutResult ($Flag: Boolean)
+begin
+  log info node 'BugTest1565' 'before';
+  if $Flag then
+    call microflow BugTest1565.SubNoResult() on error without rollback begin
+      log warning node 'BugTest1565' 'failed';
+    end error;
+    log info node 'BugTest1565' 'after call';
+  end if;
+  log info node 'BugTest1565' 'end';
+end;
+
+-- Rejoins at 'after call', inside the branch that returns.
+create microflow BugTest1565.ReturningBranch ($Flag: Boolean) returns Boolean
+begin
+  if $Flag then
+    call microflow BugTest1565.SubNoResult() on error without rollback begin
+      log warning node 'BugTest1565' 'failed';
+    end error;
+    log info node 'BugTest1565' 'after call';
+    return true;
+  else
+    log info node 'BugTest1565' 'else';
+  end if;
+  log info node 'BugTest1565' 'end';
+  return false;
+end;
+
+-- Rejoins at 'after call' in the CASE branch.
+create microflow BugTest1565.CaseBranch ($C: BugTest1565.Color)
+begin
+  case $C
+    when Red then
+      call microflow BugTest1565.SubNoResult() on error without rollback begin
+        log warning node 'BugTest1565' 'failed';
+      end error;
+      log info node 'BugTest1565' 'after call';
+    when Green, (empty) then
+      log info node 'BugTest1565' 'green';
+  end case;
+  log info node 'BugTest1565' 'end';
+end;

mdl/executor/cmd_microflows_builder_actions.go

@@ -570,6 +570,7 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID {
 
 	savedEndsWithReturn := fb.endsWithReturn
 	allBranchesReturn := len(branches) > 0
+	var caseHandlers []pendingErrorHandlerState
 	for i, br := range branches {
 		branchY := branchYs[i]
 		fb.posX = splitX + SplitWidth + HorizontalSpacing/2
@@ -579,6 +580,7 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID {
 		lastID := model.ID("")
 		pendingCase := ""
 		var prevAnchor *ast.FlowAnchors
+		outerHandlers := fb.enterBranchErrorHandlers()
 		for j, stmt := range br.body {
 			thisAnchor := stmtOwnAnchor(stmt)
 			actID := fb.addStatement(stmt)
@@ -629,9 +631,7 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID {
 				}
 				applyUserAnchors(flow, prevAnchor, thisAnchor)
 				fb.flows = append(fb.flows, flow)
-				if fb.emptyErrorHandlerFrom == lastID {
-					fb.addPendingErrorHandlerFlowForStatement(lastID, actID, stmt, statementsReferenceVar(br.body[j+1:], fb.errorHandlerSkipVar))
-				}
+				fb.addBranchErrorHandlerFlowForStatement(lastStmtIsReturn(br.body), lastID, actID, stmt, statementsReferenceVar(br.body[j+1:], fb.errorHandlerSkipVar))
 			}
 			prevAnchor = thisAnchor
 			if fb.nextConnectionPoint != "" {
@@ -648,6 +648,7 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID {
 		}
 
 		if lastStmtIsReturn(br.body) {
+			caseHandlers = append(caseHandlers, fb.takeBranchErrorHandlers(outerHandlers)...)
 			continue
 		}
 		allBranchesReturn = false
@@ -664,7 +665,9 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID {
 				tail.DestinationConnectionIndex = mergeSideFor(fb.objectPosition(lastID), fb.objectPosition(ensureMerge().ID))
 			}
 			fb.flows = append(fb.flows, tail)
+			fb.addPendingErrorHandlerFlowForStatement(lastID, ensureMerge().ID, nil)
 		}
+		caseHandlers = append(caseHandlers, fb.takeBranchErrorHandlers(outerHandlers)...)
 	}
 
 	fb.posX = mergeX + HorizontalSpacing/2
@@ -678,6 +681,12 @@ func (fb *flowBuilder) addEnumSplit(s *ast.EnumSplitStmt) model.ID {
 		// merge — as on an IF (#767). It was accepted and dropped (#1587).
 		fb.nextFlowAnchor = mergeExitFlowAnchor(s)
 	}
+	// The flow leaving the split: see queueBranchErrorHandlersAt.
+	exitID := splitID
+	if !allBranchesReturn {
+		exitID = ensureMerge().ID
+	}
+	fb.queueBranchErrorHandlersAt(caseHandlers, exitID)
 	return splitID
 }
 
@@ -748,11 +757,15 @@ func (fb *flowBuilder) addStructuredInheritanceSplit(s *ast.InheritanceSplitStmt
 		fromSplit bool
 		order     int
 		anchor    *ast.FlowAnchors
+		// handlers are the branch's error handlers still to rejoin, on the
+		// flow from id to the merge.
+		handlers []pendingErrorHandlerState
 	}
 	var branchTails []branchTail
 
 	savedEndsWithReturn := fb.endsWithReturn
 	allBranchesReturn := len(s.Cases) > 0 && len(s.ElseBody) > 0
+	var splitHandlers []pendingErrorHandlerState
 	branchIndex := 0
 
 	addBranch := func(caseValue string, body []ast.MicroflowStatement) {
@@ -772,7 +785,8 @@ func (fb *flowBuilder) addStructuredInheritanceSplit(s *ast.InheritanceSplitStmt
 		var lastID model.ID
 		var prevAnchor *ast.FlowAnchors
 		pendingCase := ""
-		for _, stmt := range body {
+		outerHandlers := fb.enterBranchErrorHandlers()
+		for j, stmt := range body {
 			thisAnchor := stmtOwnAnchor(stmt)
 			actID := fb.addStatement(stmt)
 			if fb.takeBranchJoin(lastID, splitID, caseValue, pendingCase, prevAnchor, nil) {
@@ -808,6 +822,7 @@ func (fb *flowBuilder) addStructuredInheritanceSplit(s *ast.InheritanceSplitStmt
 					applyUserAnchors(flow, prevAnchor, thisAnchor)
 					fb.flows = append(fb.flows, flow)
 				}
+				fb.addBranchErrorHandlerFlowForStatement(lastStmtIsReturn(body), lastID, actID, stmt, statementsReferenceVar(body[j+1:], fb.errorHandlerSkipVar))
 			}
 			prevAnchor = thisAnchor
 			if fb.nextConnectionPoint != "" {
@@ -826,9 +841,11 @@ func (fb *flowBuilder) addStructuredInheritanceSplit(s *ast.InheritanceSplitStmt
 		if !lastStmtIsReturn(body) {
 			allBranchesReturn = false
 			if lastID != "" {
-				branchTails = append(branchTails, branchTail{id: lastID, caseValue: pendingCase, anchor: prevAnchor})
+				branchTails = append(branchTails, branchTail{id: lastID, caseValue: pendingCase, anchor: prevAnchor, handlers: fb.takeBranchErrorHandlers(outerHandlers)})
+				return
 			}
 		}
+		splitHandlers = append(splitHandlers, fb.takeBranchErrorHandlers(outerHandlers)...)
 	}
 
 	for _, c := range s.Cases {
@@ -864,6 +881,8 @@ func (fb *flowBuilder) addStructuredInheritanceSplit(s *ast.InheritanceSplitStmt
 	fb.posX = mergeX + MergeSize + HorizontalSpacing/2
 	fb.posY = centerY
 	fb.endsWithReturn = savedEndsWithReturn
+	// The flow leaving the split: see queueBranchErrorHandlersAt.
+	exitID := splitID
 	if allBranchesReturn {
 		fb.endsWithReturn = true
 	} else if len(branchTails) > 0 {
@@ -895,10 +914,13 @@ func (fb *flowBuilder) addStructuredInheritanceSplit(s *ast.InheritanceSplitStmt
 					applyUserAnchors(flow, tail.anchor, nil)
 					fb.flows = append(fb.flows, flow)
 				}
+				splitHandlers = append(splitHandlers, fb.settleBranchErrorHandlersOn(tail.handlers, tail.id, merge.ID)...)
 			}
 		}
 		fb.nextConnectionPoint = merge.ID
+		exitID = merge.ID
 	}
+	fb.queueBranchErrorHandlersAt(splitHandlers, exitID)
 	return splitID
 }