workflow-core icon indicating copy to clipboard operation
workflow-core copied to clipboard

Wrong Workflow status when Decide Step fails

Open moisesisaias opened this issue 4 years ago • 3 comments

Hey!

Before anything I would like express that this is a really great project, thank you very much for it!

We are using the Decide step to make a workflow follow one of two paths based on a condition, but in some cases that condition cannot be evaluated (invalid data or condition structure), therefore we want the Decide step to fail. The issue with making the Decide step fail while evaluating the condition is that the workflow status ends up being completed instead of terminated or suspended because the pointer has been already set as inactive by the moment we throw the exception (please see this line) and the DetermineNextExecutionTime method in WorkflowExecutor is setting the workflow status as completed because no step/pointer is active, sleeping or missing, so that's overwriting the already set error status in the workflow.

We are throwing the exception here, inside the custom method the Matches method is invoking.

We suggest changing

           if (result.Proceed)
            {
                pointer.Active = false;
                pointer.EndTime = _datetimeProvider.UtcNow;
                pointer.Status = PointerStatus.Complete;
                                
                foreach (var outcomeTarget in step.Outcomes.Where(x => x.Matches(result, workflow.Data)))
                {                    
                    workflow.ExecutionPointers.Add(_pointerFactory.BuildNextPointer(def, pointer, outcomeTarget));
                }

to

           if (result.Proceed)
            {
                foreach (var outcomeTarget in step.Outcomes.Where(x => x.Matches(result, workflow.Data)))
                {                    
                    workflow.ExecutionPointers.Add(_pointerFactory.BuildNextPointer(def, pointer, outcomeTarget));
                }

                pointer.Active = false;
                pointer.EndTime = _datetimeProvider.UtcNow;
                pointer.Status = PointerStatus.Complete;

, but we are not 100% sure that will not affect anything else.

Please let us know if you need anything else about this or if it can be fixed/addressed differently.

Also, I could submit a pull request if you are okay with the suggestion and busy.

moisesisaias avatar Jan 08 '21 14:01 moisesisaias

I'd like to chime in on this issue as I'm seeing similar behavior.

My workflow's pseudocode is like this:

StartWith<TaskA>
Saga
	StartWith<TaskB>
	Then<TaskC>
	Decide
		BranchA -> StartWith<TaskD>
		BranchB -> StartWith<TaskE>
	Then<TaskF>
EndWorkflow

If TaskD throws an exception TaskF (and the others after the Decide) get executed - definitely not what I want.

DavidPx avatar Jan 14 '21 22:01 DavidPx

Hey Dan, I got what I believe to be a repro case for this bug, see the linked commit.

Notice that my branches only have one path... kinda dumb. In my project I switched to using the If step which does not exhibit this bug. I hope that's a helpful hint for figuring this out!

DavidPx avatar Feb 26 '21 15:02 DavidPx

Hey Folks, is anyone working on this?

ChrisBertrand avatar Mar 22 '23 11:03 ChrisBertrand