elsa-workflows / elsa-core

A .NET workflows library
https://v3.elsaworkflows.io/
MIT License
6.26k stars 1.15k forks source link

`GetNamedActivityPropertyAsync()` throws NRE when called on `CompositeActivity` #2702

Open ann-shakun opened 2 years ago

ann-shakun commented 2 years ago

Hi!

When using CompositeActivity it seems it's not possible to access activity output by calling GetNamedActivityPropertyAsync(). If you move A1 and A2 to the parent workflow, it works as expected. Am I missing something?

Reproduced in 2.4.1 and 2.5.0-preview.4.

public class TestWorkflow : IWorkflow
{
    public void Build(IWorkflowBuilder builder)
    {
        builder
            // any trigger
            .Then<A0>();
    }
}

public class A0 : CompositeActivity
{
    public override void Build(ICompositeActivityBuilder builder)
    {
        builder
            .Then<A1>().WithName(nameof(A1))
            .Then<A2>(s => s.Set(v => v.Input,
                async p => await p.GetNamedActivityPropertyAsync<A1, string>(nameof(A1),
                    a => a.Result)));
    }
}

public class A1 : Activity
{
    [ActivityOutput]
    public string Result { get; set; }

    protected override IActivityExecutionResult OnExecute()
    {
        Result = "test";
        return base.OnExecute();
    }
}

public class A2 : Activity
{
    [ActivityInput]
    public string Input
    {
        get => GetState<string>();
        set => SetState(value);
    }

    protected override IActivityExecutionResult OnExecute()
    {
        // do smth
        return base.OnExecute();
    }
}

Logs:

warn: Elsa.Services.Workflows.WorkflowRunner[0]
      Failed to run workflow 4baf2f5b63604b52864afad3daafaeb1
      Elsa.Exceptions.CannotSetActivityPropertyValueException: An exception was thrown whilst setting 'A2.Input'. See the inner exception for further details.
       ---> System.NullReferenceException: Object reference not set to an instance of an object.
         at Elsa.Services.Models.WorkflowExecutionContext.GetActivityPropertyAsync(IActivityBlueprint activityBlueprint, String propertyName, CancellationToken cancellationToken)
         at Elsa.Services.Models.WorkflowExecutionContext.GetNamedActivityPropertyAsync(String activityName, String propertyName, CancellationToken cancellationToken)
         at Elsa.Services.Models.WorkflowExecutionContext.GetNamedActivityPropertyAsync[T](String activityName, String propertyName, CancellationToken cancellationToken)
         at Elsa.Services.Models.WorkflowExecutionContext.GetNamedActivityPropertyAsync[TActivity,T](String activityName, Expression`1 propertyExpression, CancellationToken cancellationToken)
         at Elsa.Services.Models.ActivityExecutionContext.GetNamedActivityPropertyAsync[TActivity,T](String activityName, Expression`1 propertyExpression, CancellationToken cancellationToken)
         at ElsaWf.A0.<>c.<<Build>b__0_2>d.MoveNext() in <path>TestWorkflow.cs:line 28
      --- End of stack trace from previous location ---
         at Elsa.Builders.SetupActivity`1.<>c__DisplayClass6_0`1.<<Set>b__0>d.MoveNext()
      --- End of stack trace from previous location ---
         at Elsa.Services.Workflows.DelegateActivityPropertyValueProvider.GetValueAsync(ActivityExecutionContext context, CancellationToken cancellationToken)
         at Elsa.Services.ActivityPropertyProviders.SetActivityPropertiesAsync(IActivity activity, ActivityExecutionContext activityExecutionContext, CancellationToken cancellationToken)
         --- End of inner exception stack trace ---
         at Elsa.Services.ActivityPropertyProviders.SetActivityPropertiesAsync(IActivity activity, ActivityExecutionContext activityExecutionContext, CancellationToken cancellationToken)
         at Elsa.Services.Workflows.ActivityActivator.ActivateActivityAsync(ActivityExecutionContext context, Type type, CancellationToken cancellationToken)
         at Elsa.Providers.ActivityTypes.TypeBasedActivityProvider.<>c__DisplayClass6_0.<<CreateActivityTypeAsync>b__0>d.MoveNext()
      --- End of stack trace from previous location ---
         at Elsa.Services.Workflows.WorkflowRunner.RunCoreAsync(WorkflowExecutionContext workflowExecutionContext, ActivityOperation activityOperation, CancellationToken cancellationToken)
         at Elsa.Services.Workflows.WorkflowRunner.RunCoreAsync(WorkflowExecutionContext workflowExecutionContext, ActivityOperation activityOperation, CancellationToken cancellationToken)
         at Elsa.Services.Workflows.WorkflowRunner.RunAsync(WorkflowExecutionContext workflowExecutionContext, ActivityOperation activityOperation, CancellationToken cancellationToken)
ann-shakun commented 2 years ago

@sfmskywalker, I was looking into the source code and it seems like I found the reason of NRE.

https://github.com/elsa-workflows/elsa-core/blob/f3d64e3ac56198e1c73dd5212f2ec5b4b88d6c1c/src/core/Elsa.Abstractions/Services/Models/ActivityExecutionContext.cs#L252-L266

In my scenario container.Parent returned from GetCompositeName() is always null because A0 is inherited from CompositeActivity. Also valid for nested composite activities. So, instead of activity-1:A1 it returns A1. But I don't understand how to distinguish between named activity declared inside CompositeActivity and a CompositeActivity itself, because in both cases Parent is null.

sfmskywalker commented 2 years ago

Hi @ann-shakun , thanks for reporting this and the details you provided. We'll try and get this fixed for 2.5.1.