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

forach in code can result in an exception

Open FransVanEk opened this issue 3 years ago • 3 comments

The Exception

Elsa.Exceptions.TypeConversionException
  HResult=0x80131500
  Message=Failed to convert an object of type Workflows.ForEachIssue+GridAreaTest to System.Collections.Generic.List`1[Workflows.ForEachIssue+GridAreaTest]
  Source=Elsa.Abstractions
  StackTrace:
   at Elsa.ObjectConverter.ConvertTo(Object value, Type targetType)
   at Elsa.ObjectConverter.ConvertTo[T](Object value)
   at Elsa.Services.Models.ActivityExecutionContext.GetInput[T]()
   at Workflows.ForEachIssue.<>c.<Build>b__0_0(ActivityExecutionContext context) in 
   at Elsa.Activities.ControlFlow.ForEachBuilderExtensions.<>c__DisplayClass1_0`1.<ForEach>b__2(ActivityExecutionContext context)
   at Elsa.Builders.SetupActivityExtensions.<>c__DisplayClass6_0`2.<Set>b__0(ActivityExecutionContext context)
   at Elsa.Builders.SetupActivity`1.<>c__DisplayClass6_0`1.<<Set>b__0>d.MoveNext()

  This exception was originally thrown at this call stack:
    [External Code]

Inner Exception 1:
InvalidCastException: Object must implement IConvertible.

Workflow

  public class ForEachIssue : IWorkflow
    {
        public void Build(IWorkflowBuilder builder)
        {
            _ = builder.
                SignalReceived("foreachIssue")
                .ForEach(context => context.GetInput<List<GridAreaTest>>(), item =>
                     {
                         item.SendSignal(signal =>
                            signal
                                  .WithSignal("issue")
                                  .WithInput(context => context.Input));
                     }).WithDisplayName($"foreach grid area");
        }

        public class GridAreaTest
        {
            public string Id { get; set; } = string.Empty;
        }
    }

Input for the signal

{
    "correlationId": "demo",
    "input": [
        {
            "id": "6"
        },
        {
            "id": "demo"
        }
    ]
}

FransVanEk avatar Feb 03 '22 20:02 FransVanEk

I see now. The issue is that during the second iteration (when the ForEach executes for the second time), the engine evaluates the ForEach collection initializer again, using this expression: context.GetInput<List<GridAreaTest>>(). Only this second time, the input is the previous item that the ForEach activity returned as its output. Which is pretty dumb - why would it set the current item as its output 🤷🏻‍♂️

Regardless of that, it would also be an issue if you had any other activity on the iterate branch that itself returns an output. The last output would be used as the input when executing the ForEach activity again.

The real issue is therefore that input & output aren't scoped within the ForEach activity.

To work around this, we need to basically copy the initial input (the list of GridAreaTest) into a variable, and let the ForEach activity use that variable instead of evaluating the context.GetInput<List<GridAreaTest>>() expression for each iteration (where input is the output of the last executed activity, which isn't the SignalReceived activity).

The following workflow will work:

public void Build(IWorkflowBuilder builder)
{
builder
    .SignalReceived("foreachIssue")

    // Store the output from the SignalReceived activity into a variable.
    .SetVariable("items", context => context.GetInput<List<GridAreaTest>>())

    // Iterate on the value of the "items" variable created in the previous step.
    .ForEach(context => context.GetVariable<List<GridAreaTest>>("items")!, item =>
    {
        item.SendSignal(signal =>
            signal
                .WithSignal("issue")
                .WithInput(context => context.Input));
    }).WithDisplayName($"foreach grid area");
}

sfmskywalker avatar Feb 03 '22 21:02 sfmskywalker

thanks, the workaround worked for me.

FransVanEk avatar Feb 04 '22 07:02 FransVanEk

I think it would be nice to change the title of this issue to "Can't pass activity's output collection as items to ForEach" for easier referencing/search.

montoner0 avatar May 25 '22 13:05 montoner0