Skip to content

Commit b2c1563

Browse files
hjothamendixhjothamendix
committed
fix: cover all custom error handler action inputs
Symptom: custom error handlers could be routed as if their output-dependent continuation were safe when the continuation used less common action types. Root cause: the continuing-handler detector and skip-variable reference walker only covered a subset of microflow statements with ErrorHandling and input expressions. Fix: centralize statement ErrorHandling lookup, cover every current action statement with ErrorHandling, extend variable-reference detection for action arguments, REST auth, mapping, transform, download, and validation feedback inputs, and remove the vestigial errorY parameter from the skip helper. Tests: add synthetic unit coverage for the supported action statement matrix and skip-variable input detection.
1 parent 315ae6b commit b2c1563

3 files changed

Lines changed: 199 additions & 62 deletions

File tree

‎mdl/executor/cmd_microflows_builder_control.go‎

Lines changed: 45 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -453,35 +453,12 @@ func (fb *flowBuilder) addIfStatement(s *ast.IfStmt) model.ID {
453453

454454
func bodyHasContinuingCustomErrorHandler(stmts []ast.MicroflowStatement) bool {
455455
for _, stmt := range stmts {
456-
switch s := stmt.(type) {
457-
case *ast.CallMicroflowStmt:
458-
if isContinuingCustomErrorHandler(s.ErrorHandling) || bodyHasContinuingCustomErrorHandler(errorBody(s.ErrorHandling)) {
459-
return true
460-
}
461-
case *ast.CallJavaActionStmt:
462-
if isContinuingCustomErrorHandler(s.ErrorHandling) || bodyHasContinuingCustomErrorHandler(errorBody(s.ErrorHandling)) {
463-
return true
464-
}
465-
case *ast.RestCallStmt:
466-
if isContinuingCustomErrorHandler(s.ErrorHandling) || bodyHasContinuingCustomErrorHandler(errorBody(s.ErrorHandling)) {
467-
return true
468-
}
469-
case *ast.ImportFromMappingStmt:
470-
if isContinuingCustomErrorHandler(s.ErrorHandling) || bodyHasContinuingCustomErrorHandler(errorBody(s.ErrorHandling)) {
471-
return true
472-
}
473-
case *ast.CreateObjectStmt:
474-
if isContinuingCustomErrorHandler(s.ErrorHandling) || bodyHasContinuingCustomErrorHandler(errorBody(s.ErrorHandling)) {
475-
return true
476-
}
477-
case *ast.MfCommitStmt:
478-
if isContinuingCustomErrorHandler(s.ErrorHandling) || bodyHasContinuingCustomErrorHandler(errorBody(s.ErrorHandling)) {
479-
return true
480-
}
481-
case *ast.DeleteObjectStmt:
482-
if isContinuingCustomErrorHandler(s.ErrorHandling) || bodyHasContinuingCustomErrorHandler(errorBody(s.ErrorHandling)) {
456+
if eh := statementErrorHandling(stmt); eh != nil {
457+
if isContinuingCustomErrorHandler(eh) || bodyHasContinuingCustomErrorHandler(eh.Body) {
483458
return true
484459
}
460+
}
461+
switch s := stmt.(type) {
485462
case *ast.IfStmt:
486463
if bodyHasContinuingCustomErrorHandler(s.ThenBody) || bodyHasContinuingCustomErrorHandler(s.ElseBody) {
487464
return true
@@ -665,6 +642,47 @@ func containsBreakForCurrentLoop(stmts []ast.MicroflowStatement) bool {
665642
return false
666643
}
667644

645+
func statementErrorHandling(stmt ast.MicroflowStatement) *ast.ErrorHandlingClause {
646+
switch s := stmt.(type) {
647+
case *ast.RetrieveStmt:
648+
return s.ErrorHandling
649+
case *ast.CreateObjectStmt:
650+
return s.ErrorHandling
651+
case *ast.MfCommitStmt:
652+
return s.ErrorHandling
653+
case *ast.DeleteObjectStmt:
654+
return s.ErrorHandling
655+
case *ast.CallMicroflowStmt:
656+
return s.ErrorHandling
657+
case *ast.CallNanoflowStmt:
658+
return s.ErrorHandling
659+
case *ast.CallJavaActionStmt:
660+
return s.ErrorHandling
661+
case *ast.CallJavaScriptActionStmt:
662+
return s.ErrorHandling
663+
case *ast.CallWebServiceStmt:
664+
return s.ErrorHandling
665+
case *ast.ExecuteDatabaseQueryStmt:
666+
return s.ErrorHandling
667+
case *ast.CallExternalActionStmt:
668+
return s.ErrorHandling
669+
case *ast.DownloadFileStmt:
670+
return s.ErrorHandling
671+
case *ast.RestCallStmt:
672+
return s.ErrorHandling
673+
case *ast.SendRestRequestStmt:
674+
return s.ErrorHandling
675+
case *ast.ImportFromMappingStmt:
676+
return s.ErrorHandling
677+
case *ast.ExportToMappingStmt:
678+
return s.ErrorHandling
679+
case *ast.TransformJsonStmt:
680+
return s.ErrorHandling
681+
default:
682+
return nil
683+
}
684+
}
685+
668686
func containsContinueStmt(stmts []ast.MicroflowStatement) bool {
669687
for _, stmt := range stmts {
670688
switch s := stmt.(type) {

‎mdl/executor/cmd_microflows_builder_flows.go‎

Lines changed: 83 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,8 @@ func (fb *flowBuilder) finishCustomErrorHandler(activityID model.ID, activityX i
5151
return
5252
}
5353
if len(eh.Body) > 0 {
54-
errorY := fb.posY + VerticalSpacing
5554
mergeID := fb.addErrorHandlerFlow(activityID, activityX, eh.Body)
56-
fb.handleErrorHandlerMergeWithSkip(mergeID, activityID, errorY, outputVar)
55+
fb.handleErrorHandlerMergeWithSkip(mergeID, activityID, outputVar)
5756
return
5857
}
5958
fb.registerEmptyCustomErrorHandlerWithSkip(activityID, eh, outputVar)
@@ -380,6 +379,22 @@ func statementsReferenceVar(stmts []ast.MicroflowStatement, varName string) bool
380379
return false
381380
}
382381

382+
func callArgumentVarRefs(args []ast.CallArgument) []string {
383+
var refs []string
384+
for _, arg := range args {
385+
refs = append(refs, exprVarRefs(arg.Value)...)
386+
}
387+
return refs
388+
}
389+
390+
func templateParamVarRefs(params []ast.TemplateParam) []string {
391+
var refs []string
392+
for _, param := range params {
393+
refs = append(refs, exprVarRefs(param.Value)...)
394+
}
395+
return refs
396+
}
397+
383398
func errorHandlerStatementVarRefs(stmt ast.MicroflowStatement) []string {
384399
var refs []string
385400
switch s := stmt.(type) {
@@ -390,9 +405,7 @@ func errorHandlerStatementVarRefs(stmt ast.MicroflowStatement) []string {
390405
case *ast.LogStmt:
391406
refs = append(refs, exprVarRefs(s.Node)...)
392407
refs = append(refs, exprVarRefs(s.Message)...)
393-
for _, param := range s.Template {
394-
refs = append(refs, exprVarRefs(param.Value)...)
395-
}
408+
refs = append(refs, templateParamVarRefs(s.Template)...)
396409
case *ast.ShowMessageStmt:
397410
refs = append(refs, exprVarRefs(s.Message)...)
398411
for _, arg := range s.TemplateArgs {
@@ -426,35 +439,71 @@ func errorHandlerStatementVarRefs(stmt ast.MicroflowStatement) []string {
426439
}
427440
refs = append(refs, exprVarRefs(s.Where)...)
428441
case *ast.CallMicroflowStmt:
429-
for _, arg := range s.Arguments {
430-
refs = append(refs, exprVarRefs(arg.Value)...)
431-
}
442+
refs = append(refs, callArgumentVarRefs(s.Arguments)...)
443+
case *ast.CallNanoflowStmt:
444+
refs = append(refs, callArgumentVarRefs(s.Arguments)...)
432445
case *ast.CallJavaActionStmt:
433-
for _, arg := range s.Arguments {
434-
refs = append(refs, exprVarRefs(arg.Value)...)
435-
}
446+
refs = append(refs, callArgumentVarRefs(s.Arguments)...)
447+
case *ast.CallJavaScriptActionStmt:
448+
refs = append(refs, callArgumentVarRefs(s.Arguments)...)
449+
case *ast.CallWebServiceStmt:
450+
refs = append(refs, exprVarRefs(s.Timeout)...)
451+
case *ast.ExecuteDatabaseQueryStmt:
452+
refs = append(refs, callArgumentVarRefs(s.Arguments)...)
453+
refs = append(refs, callArgumentVarRefs(s.ConnectionArguments)...)
454+
case *ast.CallExternalActionStmt:
455+
refs = append(refs, callArgumentVarRefs(s.Arguments)...)
436456
case *ast.RestCallStmt:
437457
refs = append(refs, exprVarRefs(s.URL)...)
438-
for _, param := range s.URLParams {
439-
refs = append(refs, exprVarRefs(param.Value)...)
440-
}
458+
refs = append(refs, templateParamVarRefs(s.URLParams)...)
441459
for _, header := range s.Headers {
442460
refs = append(refs, exprVarRefs(header.Value)...)
443461
}
462+
if s.Auth != nil {
463+
refs = append(refs, exprVarRefs(s.Auth.Username)...)
464+
refs = append(refs, exprVarRefs(s.Auth.Password)...)
465+
}
444466
if s.Body != nil {
445467
refs = append(refs, exprVarRefs(s.Body.Template)...)
446-
for _, param := range s.Body.TemplateParams {
447-
refs = append(refs, exprVarRefs(param.Value)...)
448-
}
468+
refs = append(refs, templateParamVarRefs(s.Body.TemplateParams)...)
449469
if s.Body.SourceVariable != "" {
450470
refs = append(refs, s.Body.SourceVariable)
451471
}
452472
}
453473
refs = append(refs, exprVarRefs(s.Timeout)...)
474+
case *ast.SendRestRequestStmt:
475+
for _, param := range s.Parameters {
476+
refs = append(refs, sourceAttributeVarRefs(param.Expression)...)
477+
}
478+
if s.BodyVariable != "" {
479+
refs = append(refs, s.BodyVariable)
480+
}
481+
case *ast.ImportFromMappingStmt:
482+
if s.SourceVariable != "" {
483+
refs = append(refs, s.SourceVariable)
484+
}
485+
case *ast.ExportToMappingStmt:
486+
if s.SourceVariable != "" {
487+
refs = append(refs, s.SourceVariable)
488+
}
489+
case *ast.TransformJsonStmt:
490+
if s.InputVariable != "" {
491+
refs = append(refs, s.InputVariable)
492+
}
454493
case *ast.MfCommitStmt:
455494
refs = append(refs, s.Variable)
456495
case *ast.DeleteObjectStmt:
457496
refs = append(refs, s.Variable)
497+
case *ast.DownloadFileStmt:
498+
refs = append(refs, s.FileDocument)
499+
case *ast.ValidationFeedbackStmt:
500+
if s.AttributePath != nil {
501+
refs = append(refs, s.AttributePath.Variable)
502+
}
503+
refs = append(refs, exprVarRefs(s.Message)...)
504+
for _, arg := range s.TemplateArgs {
505+
refs = append(refs, exprVarRefs(arg)...)
506+
}
458507
case *ast.AddToListStmt:
459508
refs = append(refs, s.Item, s.List)
460509
case *ast.RemoveFromListStmt:
@@ -514,19 +563,18 @@ func (fb *flowBuilder) addErrorHandlerFlow(sourceActivityID model.ID, sourceX in
514563

515564
// Build error handler activities
516565
errBuilder := &flowBuilder{
517-
posX: errorX,
518-
posY: errorY,
519-
baseY: errorY,
520-
spacing: HorizontalSpacing,
521-
returnType: fb.returnType,
522-
hasReturnValue: fb.hasReturnValue,
523-
varTypes: fb.varTypes,
524-
declaredVars: fb.declaredVars,
525-
measurer: fb.measurer,
526-
backend: fb.backend,
527-
hierarchy: fb.hierarchy,
528-
restServices: fb.restServices,
529-
isNanoflow: fb.isNanoflow,
566+
posX: errorX,
567+
posY: errorY,
568+
baseY: errorY,
569+
spacing: HorizontalSpacing,
570+
returnType: fb.returnType,
571+
varTypes: fb.varTypes,
572+
declaredVars: fb.declaredVars,
573+
measurer: fb.measurer,
574+
backend: fb.backend,
575+
hierarchy: fb.hierarchy,
576+
restServices: fb.restServices,
577+
isNanoflow: fb.isNanoflow,
530578
}
531579

532580
var lastErrID model.ID
@@ -571,16 +619,16 @@ func (fb *flowBuilder) addErrorHandlerFlow(sourceActivityID model.ID, sourceX in
571619

572620
// handleErrorHandlerMerge creates an EndEvent for error handlers that want to merge back.
573621
// This is a fallback until full merge support is implemented. Caller should pass
574-
// the ID returned by addErrorHandlerFlow and the error handler Y position.
575-
func (fb *flowBuilder) handleErrorHandlerMerge(lastErrID model.ID, activityID model.ID, errorY int) {
576-
fb.handleErrorHandlerMergeWithSkip(errorHandlerTail{id: lastErrID}, activityID, errorY, "")
622+
// the tail returned by addErrorHandlerFlow and the error handler Y position.
623+
func (fb *flowBuilder) handleErrorHandlerMerge(tail errorHandlerTail, activityID model.ID, errorY int) {
624+
_ = errorY
625+
fb.handleErrorHandlerMergeWithSkip(tail, activityID, "")
577626
}
578627

579-
func (fb *flowBuilder) handleErrorHandlerMergeWithSkip(tail errorHandlerTail, activityID model.ID, errorY int, skipVar string) {
628+
func (fb *flowBuilder) handleErrorHandlerMergeWithSkip(tail errorHandlerTail, activityID model.ID, skipVar string) {
580629
if tail.id == "" {
581630
return // No merge needed (error handler terminates with RETURN or RAISE ERROR)
582631
}
583-
_ = errorY
584632
fb.queueActivePendingErrorHandler()
585633
fb.errorHandlerSource = activityID
586634
fb.errorHandlerTailFrom = tail.id

‎mdl/executor/cmd_microflows_builder_terminal_test.go‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,77 @@ func TestBuildFlowGraph_ConsecutiveCustomHandlersEachRejoinsContinuation(t *test
407407
}
408408
}
409409

410+
func TestBodyHasContinuingCustomErrorHandler_CoversActionStatements(t *testing.T) {
411+
continuingHandler := &ast.ErrorHandlingClause{
412+
Type: ast.ErrorHandlingCustomWithoutRollback,
413+
Body: []ast.MicroflowStatement{
414+
&ast.LogStmt{Level: ast.LogError, Message: &ast.LiteralExpr{Kind: ast.LiteralString, Value: "failed"}},
415+
},
416+
}
417+
418+
tests := []struct {
419+
name string
420+
stmt ast.MicroflowStatement
421+
}{
422+
{name: "retrieve", stmt: &ast.RetrieveStmt{ErrorHandling: continuingHandler}},
423+
{name: "create object", stmt: &ast.CreateObjectStmt{ErrorHandling: continuingHandler}},
424+
{name: "commit", stmt: &ast.MfCommitStmt{ErrorHandling: continuingHandler}},
425+
{name: "delete", stmt: &ast.DeleteObjectStmt{ErrorHandling: continuingHandler}},
426+
{name: "call microflow", stmt: &ast.CallMicroflowStmt{ErrorHandling: continuingHandler}},
427+
{name: "call nanoflow", stmt: &ast.CallNanoflowStmt{ErrorHandling: continuingHandler}},
428+
{name: "call java action", stmt: &ast.CallJavaActionStmt{ErrorHandling: continuingHandler}},
429+
{name: "call javascript action", stmt: &ast.CallJavaScriptActionStmt{ErrorHandling: continuingHandler}},
430+
{name: "call web service", stmt: &ast.CallWebServiceStmt{ErrorHandling: continuingHandler}},
431+
{name: "execute database query", stmt: &ast.ExecuteDatabaseQueryStmt{ErrorHandling: continuingHandler}},
432+
{name: "call external action", stmt: &ast.CallExternalActionStmt{ErrorHandling: continuingHandler}},
433+
{name: "download file", stmt: &ast.DownloadFileStmt{ErrorHandling: continuingHandler}},
434+
{name: "rest call", stmt: &ast.RestCallStmt{ErrorHandling: continuingHandler}},
435+
{name: "send rest request", stmt: &ast.SendRestRequestStmt{ErrorHandling: continuingHandler}},
436+
{name: "import mapping", stmt: &ast.ImportFromMappingStmt{ErrorHandling: continuingHandler}},
437+
{name: "export mapping", stmt: &ast.ExportToMappingStmt{ErrorHandling: continuingHandler}},
438+
{name: "transform json", stmt: &ast.TransformJsonStmt{ErrorHandling: continuingHandler}},
439+
}
440+
441+
for _, tt := range tests {
442+
t.Run(tt.name, func(t *testing.T) {
443+
if !bodyHasContinuingCustomErrorHandler([]ast.MicroflowStatement{tt.stmt}) {
444+
t.Fatalf("%T must be visible to continuing custom handler detection", tt.stmt)
445+
}
446+
})
447+
}
448+
}
449+
450+
func TestStatementReferencesVar_CoversActionStatementInputs(t *testing.T) {
451+
ref := &ast.VariableExpr{Name: "SkippedOutput"}
452+
tests := []struct {
453+
name string
454+
stmt ast.MicroflowStatement
455+
}{
456+
{name: "call nanoflow", stmt: &ast.CallNanoflowStmt{Arguments: []ast.CallArgument{{Name: "value", Value: ref}}}},
457+
{name: "call javascript action", stmt: &ast.CallJavaScriptActionStmt{Arguments: []ast.CallArgument{{Name: "value", Value: ref}}}},
458+
{name: "call web service timeout", stmt: &ast.CallWebServiceStmt{Timeout: ref}},
459+
{name: "execute database query argument", stmt: &ast.ExecuteDatabaseQueryStmt{Arguments: []ast.CallArgument{{Name: "value", Value: ref}}}},
460+
{name: "execute database query connection argument", stmt: &ast.ExecuteDatabaseQueryStmt{ConnectionArguments: []ast.CallArgument{{Name: "value", Value: ref}}}},
461+
{name: "call external action", stmt: &ast.CallExternalActionStmt{Arguments: []ast.CallArgument{{Name: "value", Value: ref}}}},
462+
{name: "rest auth", stmt: &ast.RestCallStmt{Auth: &ast.RestAuth{Username: ref}}},
463+
{name: "send rest request parameter", stmt: &ast.SendRestRequestStmt{Parameters: []ast.SendRestParamDef{{Name: "value", Expression: "$SkippedOutput/Name"}}}},
464+
{name: "send rest request body", stmt: &ast.SendRestRequestStmt{BodyVariable: "SkippedOutput"}},
465+
{name: "import mapping", stmt: &ast.ImportFromMappingStmt{SourceVariable: "SkippedOutput"}},
466+
{name: "export mapping", stmt: &ast.ExportToMappingStmt{SourceVariable: "SkippedOutput"}},
467+
{name: "transform json", stmt: &ast.TransformJsonStmt{InputVariable: "SkippedOutput"}},
468+
{name: "download file", stmt: &ast.DownloadFileStmt{FileDocument: "SkippedOutput"}},
469+
{name: "validation feedback target", stmt: &ast.ValidationFeedbackStmt{AttributePath: &ast.AttributePathExpr{Variable: "SkippedOutput", Path: []string{"Name"}}}},
470+
}
471+
472+
for _, tt := range tests {
473+
t.Run(tt.name, func(t *testing.T) {
474+
if !statementReferencesVar(tt.stmt, "SkippedOutput") {
475+
t.Fatalf("%T must expose input variable references for custom handler routing", tt.stmt)
476+
}
477+
})
478+
}
479+
}
480+
410481
func TestBuildFlowGraph_EmptyOutputHandlerTerminatesBeforeOutputDependentTail(t *testing.T) {
411482
body := []ast.MicroflowStatement{
412483
&ast.CallJavaActionStmt{

0 commit comments

Comments
 (0)