Merge pull request #7199 from elsa-workflows/bugfix/read-invalid-synthetic-inputs

Bugfix: handle uncaught exception thrown in ReadSyntheticInputs

src/modules/Elsa.Dsl.ElsaScript/Compiler/ElsaScriptCompiler.cs

@@ -202,8 +202,9 @@ public class ElsaScriptCompiler(IActivityRegistryLookupService activityRegistryL
         else
         {
             // No positional arguments, use default constructor
-            var activityConstructorContext = new ActivityConstructorContext(activityDescriptor, ActivityActivator.Create);
-            activity = activityDescriptor.Constructor(activityConstructorContext);
+            var activityConstructorContext = new ActivityConstructorContext(activityDescriptor, (t) => new(ActivityActivator.Create(t)));
+            var activityConstructionResult = activityDescriptor.Constructor(activityConstructorContext);
+            activity = activityConstructionResult.Activity;
         }
 
         // Set named argument properties

src/modules/Elsa.Workflows.Core/Exceptions/InvalidActivityDescriptorInputException.cs

@@ -0,0 +1,5 @@
+namespace Elsa.Workflows.Exceptions;
+
+public sealed class InvalidActivityDescriptorInputException(string message) : Exception(message)
+{
+}

src/modules/Elsa.Workflows.Core/Models/ActivityConstructionResult.cs

@@ -0,0 +1,17 @@
+namespace Elsa.Workflows.Models;
+
+public class ActivityConstructionResult(IActivity activity, IEnumerable<Exception>? exceptions = null)
+{
+    public bool HasExceptions => Exceptions.Any();
+
+    public IActivity Activity { get; } = activity;
+    public IEnumerable<Exception> Exceptions { get; } = exceptions ?? [];
+
+    public ActivityConstructionResult<TActivity> Cast<TActivity>() where TActivity : IActivity => new ((TActivity)Activity, Exceptions);
+}
+
+public sealed class ActivityConstructionResult<TActivity>(TActivity activity, IEnumerable<Exception>? exceptions = null) : ActivityConstructionResult(activity, exceptions)
+    where TActivity : IActivity
+{ 
+    public new TActivity Activity { get; } = activity;
+}

src/modules/Elsa.Workflows.Core/Models/ActivityConstructorContext.cs

@@ -3,15 +3,17 @@ using System.Text.Json.Nodes;
 using Elsa.Expressions.Helpers;
 using Elsa.Extensions;
 using Elsa.Workflows.Attributes;
+using Elsa.Workflows.Exceptions;
 using Elsa.Workflows.Memory;
 using Humanizer;
+using Microsoft.Extensions.Logging;
 
 namespace Elsa.Workflows.Models;
 
-public record ActivityConstructorContext(ActivityDescriptor ActivityDescriptor, Func<Type, IActivity> ActivityFactory)
+public record ActivityConstructorContext(ActivityDescriptor ActivityDescriptor, Func<Type, ActivityConstructionResult> ActivityFactory)
 {
-    public T CreateActivity<T>() where T : IActivity => (T)ActivityFactory(typeof(T));
-    public IActivity CreateActivity(Type type) => ActivityFactory(type);
+    public ActivityConstructionResult<T> CreateActivity<T>() where T : IActivity => ActivityFactory(typeof(T)).Cast<T>();
+    public ActivityConstructionResult CreateActivity(Type type) => ActivityFactory(type);
 }
 
 public static class JsonActivityConstructorContextHelper
@@ -21,13 +23,15 @@ public static class JsonActivityConstructorContextHelper
         return new ActivityConstructorContext(activityDescriptor, type => CreateActivity(activityDescriptor, type, element, serializerOptions));
     }
 
-    public static T CreateActivity<T>(ActivityDescriptor activityDescriptor, JsonElement element, JsonSerializerOptions serializerOptions) where T : IActivity
+    public static ActivityConstructionResult<T> CreateActivity<T>(ActivityDescriptor activityDescriptor, JsonElement element, JsonSerializerOptions serializerOptions) where T : IActivity
     {
-        return (T)CreateActivity(activityDescriptor, typeof(T), element, serializerOptions);
+        return CreateActivity(activityDescriptor, typeof(T), element, serializerOptions).Cast<T>();
     }
 
-    public static IActivity CreateActivity(ActivityDescriptor activityDescriptor, Type type, JsonElement element, JsonSerializerOptions serializerOptions)
+    public static ActivityConstructionResult CreateActivity(ActivityDescriptor activityDescriptor, Type type, JsonElement element, JsonSerializerOptions serializerOptions)
     {
+        var exceptions = new List<Exception>();
+
         // 1) Grab the raw text
         var raw = element.GetRawText();
 
@@ -56,14 +60,14 @@ public static class JsonActivityConstructorContextHelper
             composite.Setup();
 
         // 9) Existing synthetic inputs/outputs routines, using the cleanedElement
-        ReadSyntheticInputs(activityDescriptor, activity, cleanedElement, serializerOptions);
+        ReadSyntheticInputs(activityDescriptor, activity, cleanedElement, serializerOptions, exceptions);
         ReadSyntheticOutputs(activityDescriptor, activity, cleanedElement);
 
         // 10) Finally re‑apply those flags
         activity.SetCanStartWorkflow(canStartWorkflow);
         activity.SetRunAsynchronously(runAsynchronously);
 
-        return activity;
+        return new(activity, exceptions);
     }
 
     /// <summary>
@@ -140,7 +144,7 @@ public static class JsonActivityConstructorContextHelper
         }
     }
 
-    private static void ReadSyntheticInputs(ActivityDescriptor activityDescriptor, IActivity activity, JsonElement activityRoot, JsonSerializerOptions options)
+    private static void ReadSyntheticInputs(ActivityDescriptor activityDescriptor, IActivity activity, JsonElement activityRoot, JsonSerializerOptions options, List<Exception> exceptions)
     {
         foreach (var inputDescriptor in activityDescriptor.Inputs.Where(x => x.IsSynthetic))
         {
@@ -152,6 +156,15 @@ public static class JsonActivityConstructorContextHelper
             if (!activityRoot.TryGetProperty(propertyName, out var propertyElement) || propertyElement.ValueKind == JsonValueKind.Null || propertyElement.ValueKind == JsonValueKind.Undefined)
                 continue;
 
+            if(propertyElement.ValueKind == JsonValueKind.Object && !propertyElement.TryGetProperty("typeName", out var _))
+            {
+                var exception = new InvalidActivityDescriptorInputException(
+                    $"Activity descriptor '{activityDescriptor.Name}' has invalid input property '{propertyName}'; missing required property 'typeName'"
+                );
+                exceptions.Add(exception);
+                continue;
+            }
+
             var isWrapped = propertyElement.ValueKind == JsonValueKind.Object && propertyElement.GetProperty("typeName").ValueKind != JsonValueKind.Undefined;
 
             if (isWrapped)

src/modules/Elsa.Workflows.Core/Models/ActivityDescriptor.cs

@@ -69,7 +69,7 @@ public class ActivityDescriptor
     /// Instantiates a concrete instance of an <see cref="IActivity"/>.
     /// </summary>
     [JsonIgnore]
-    public Func<ActivityConstructorContext, IActivity> Constructor { get; set; } = null!;
+    public Func<ActivityConstructorContext, ActivityConstructionResult> Constructor { get; set; } = null!;
 
     /// <summary>
     /// The kind of activity.

src/modules/Elsa.Workflows.Core/Serialization/Converters/ActivityJsonConverter.cs

@@ -7,6 +7,7 @@ using Elsa.Workflows.Helpers;
 using Elsa.Workflows.Models;
 using Elsa.Workflows.Serialization.Helpers;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
 
 namespace Elsa.Workflows.Serialization.Converters;
 
@@ -38,13 +39,14 @@ public class ActivityJsonConverter(
         }
 
         var clonedOptions = GetClonedOptions(options);
-
         // If the activity type is not found, create a NotFoundActivity instead.
         if (activityDescriptor == null)
         {
             var notFoundActivityDescriptor = activityRegistry.Find<NotFoundActivity>()!;
-            var notFoundActivity = JsonActivityConstructorContextHelper.CreateActivity<NotFoundActivity>(notFoundActivityDescriptor, activityRoot, clonedOptions);
+            var notFoundActivityResult = JsonActivityConstructorContextHelper.CreateActivity<NotFoundActivity>(notFoundActivityDescriptor, activityRoot, clonedOptions);
+            LogExceptionsIfAny(notFoundActivityResult);
 
+            var notFoundActivity = notFoundActivityResult.Activity;
             notFoundActivity.Type = notFoundActivityTypeName;
             notFoundActivity.Version = 1;
             notFoundActivity.MissingTypeName = activityTypeName;
@@ -56,8 +58,20 @@ public class ActivityJsonConverter(
         }
 
         var context = JsonActivityConstructorContextHelper.Create(activityDescriptor, activityRoot, clonedOptions);
-        var activity = activityDescriptor.Constructor(context);
-        return activity;
+        var activityResult = activityDescriptor.Constructor(context);
+        LogExceptionsIfAny(activityResult);
+
+        return activityResult.Activity;
+    }
+
+    void LogExceptionsIfAny(ActivityConstructionResult result)
+    {
+        if (!result.HasExceptions)
+            return;
+
+        var logger = serviceProvider.GetRequiredService<ILogger<ActivityJsonConverter>>();
+        foreach (var exception in result.Exceptions)
+            logger.LogWarning("An exception was thrown while constructing activity with id '{activityId}': {Message}", result.Activity.Id, exception.Message);
     }
 
     /// <inheritdoc />

src/modules/Elsa.Workflows.Core/Services/ActivityDescriber.cs

@@ -92,9 +92,9 @@ public class ActivityDescriber(IPropertyDefaultValueResolver defaultValueResolve
             },
             Constructor = context =>
             {
-                var activity = context.CreateActivity(activityType);
-                activity.Type = fullTypeName;
-                return activity;
+                var activityResult = context.CreateActivity(activityType);
+                activityResult.Activity.Type = fullTypeName;
+                return activityResult;
             },
         };
 

src/modules/Elsa.Workflows.Core/Services/ActivityFactory.cs

@@ -8,6 +8,6 @@ public class ActivityFactory : IActivityFactory
     /// <inheritdoc />
     public IActivity Create(Type type, ActivityConstructorContext context)
     {
-        return context.CreateActivity(type);
+        return context.CreateActivity(type).Activity;
     }
 }

src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivityDescriptorFactory.cs

@@ -57,7 +57,8 @@ public class WorkflowDefinitionActivityDescriptorFactory
             },
             Constructor = context =>
             {
-                var activity = context.CreateActivity<WorkflowDefinitionActivity>();
+                var activityResult = context.CreateActivity<WorkflowDefinitionActivity>();
+                var activity = activityResult.Activity;
                 activity.Type = typeName;
                 activity.WorkflowDefinitionId = definition.DefinitionId;
                 activity.WorkflowDefinitionVersionId = definition.Id;
@@ -65,7 +66,7 @@ public class WorkflowDefinitionActivityDescriptorFactory
                 activity.LatestAvailablePublishedVersion = latestPublishedDefinition?.Version ?? definition.Version;
                 activity.LatestAvailablePublishedVersionId = latestPublishedDefinition?.Id ?? definition.Id;
 
-                return activity;
+                return activityResult;
             }
         };
     }

src/modules/Elsa.Workflows.Management/Services/HostMethodActivityDescriber.cs

@@ -57,12 +57,13 @@ public class HostMethodActivityDescriber(IActivityDescriber activityDescriber) :
 
         descriptor.Constructor = context =>
         {
-            var activity = context.CreateActivity<HostMethodActivity>();
+            var activityResult = context.CreateActivity<HostMethodActivity>();
+            var activity = activityResult.Activity;
             activity.Type = activityTypeName;
             activity.HostType = hostType;
             activity.MethodName = methodName;
             activity.RunAsynchronously ??= descriptor.RunAsynchronously;
-            return activity;
+            return activityResult;
         };
 
         descriptor.Inputs.Clear();

test/unit/Elsa.Workflows.Core.UnitTests/Models/JsonActivityConstructorContextHelperTests.cs

@@ -1,6 +1,7 @@
 
 using System.Text.Json;
 using System.Text.Json.Serialization;
+using Elsa.Workflows.Exceptions;
 using Elsa.Workflows.Models;
 
 namespace Elsa.Workflows.Core.UnitTests.Models;
@@ -14,7 +15,7 @@ public sealed class JsonActivityConstructorContextHelperTests
     [InlineData("Metadata")]
     [InlineData("_CustomProperties")]
     [InlineData("CustomProperties")]
-    public void When_InputName_Is_ReservedKeyWord_Then_ThrowsException_WhenReadingInputs(string reservedInputName)
+    public void When_InputName_Is_ReservedKeyWord_Then_AddsException_WhenReadingInputs(string reservedInputName)
     {
         // Arrange
         var activityDescriptor = new ActivityDescriptor
@@ -31,15 +32,15 @@ public sealed class JsonActivityConstructorContextHelperTests
         };
         var jsonElement = GetJsonElementWithReservedInputName(reservedInputName);
 
-        // Act & Assert
-        Assert.Throws<KeyNotFoundException>(() =>
-        {
-            _ = JsonActivityConstructorContextHelper.CreateActivity<WorkflowAsActivity>(
-                activityDescriptor,
-                jsonElement,
-                _serializerOptions
-            );
-        });
+        // Act
+        var result = JsonActivityConstructorContextHelper.CreateActivity<WorkflowAsActivity>(
+            activityDescriptor,
+            jsonElement,
+            _serializerOptions
+        );
+
+        // Assert
+        Assert.Single(result.Exceptions, e => e is InvalidActivityDescriptorInputException);
     }
 
     private static JsonElement GetJsonElementWithReservedInputName(string inputName)

test/unit/Elsa.Workflows.Core.UnitTests/Serialization/Converters/ActivityJsonConverterTests.cs

@@ -125,7 +125,7 @@ public sealed class ActivityJsonConverterTests
 
     static IActivityRegistry CreateActivityRegistry(string typeName, IActivity activity, int? version = null)
     {
-        var descriptor = new ActivityDescriptor { Constructor = _ => activity };
+        var descriptor = new ActivityDescriptor { Constructor = _ => new(activity) };
         var activityRegistry = Substitute.For<IActivityRegistry>();
 
         if (version.HasValue)
@@ -140,7 +140,7 @@ public sealed class ActivityJsonConverterTests
     {
         var descriptor = new ActivityDescriptor
         {
-            Constructor = _ => WorkflowAsActivity,
+            Constructor = _ => new(WorkflowAsActivity),
             CustomProperties = { [customPropertyName] = customPropertyValue }
         };
 
@@ -334,6 +334,11 @@ public sealed class ActivityJsonConverterTests
     private static ActivityJsonConverter CreateSut(IActivityRegistry activityRegistry)
     {
         var expressionDescriptorRegistry = new ExpressionDescriptorRegistry([]);
+        var serviceProvider = Substitute.For<IServiceProvider>();
+        serviceProvider
+            .GetService(typeof(ILogger<ActivityJsonConverter>))!
+            .Returns(CreateLogger<ActivityJsonConverter>());
+
         return new(
             activityRegistry,
             expressionDescriptorRegistry,
@@ -342,7 +347,7 @@ public sealed class ActivityJsonConverterTests
                 new(expressionDescriptorRegistry),
                 CreateLogger<ActivityWriter>()
             ),
-            Substitute.For<IServiceProvider>()
+            serviceProvider
         );
     }