From d0c7653ccfd412eb6f63def3c6f1faf51fa0c194 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 14 Sep 2026 05:07:59 +0200 Subject: [PATCH] fix(workflows): preserve last-good variable on storage serialize failure (#8139) Closes #8129. WorkflowInstanceStorageDriver: serialize failure keeps last-good value (no destructive Remove); read convert failure returns null / StrictMode throws instead of poison untyped JsonNode. --- .../WorkflowInstanceStorageDriver.cs | 16 +- .../WorkflowInstanceStorageDriverTests.cs | 137 ++++++++++++++++++ 2 files changed, 148 insertions(+), 5 deletions(-) create mode 100644 test/unit/Elsa.Workflows.Core.UnitTests/VariableStorageDrivers/WorkflowInstanceStorageDriverTests.cs diff --git a/src/modules/Elsa.Workflows.Core/VariableStorageDrivers/WorkflowInstanceStorageDriver.cs b/src/modules/Elsa.Workflows.Core/VariableStorageDrivers/WorkflowInstanceStorageDriver.cs index ca0f53c00..7c1d4e963 100644 --- a/src/modules/Elsa.Workflows.Core/VariableStorageDrivers/WorkflowInstanceStorageDriver.cs +++ b/src/modules/Elsa.Workflows.Core/VariableStorageDrivers/WorkflowInstanceStorageDriver.cs @@ -37,10 +37,8 @@ public class WorkflowInstanceStorageDriver(IPayloadSerializer payloadSerializer, } catch (Exception ex) when (ex is JsonException or NotSupportedException or ObjectDisposedException) { - logger.LogWarning(ex, "Failed to serialize variable '{VariableId}' of type '{VariableType}' for workflow instance storage. The variable will be skipped.", + logger.LogWarning(ex, "Failed to serialize variable '{VariableId}' of type '{VariableType}' for workflow instance storage. The stored value was left unchanged.", id, value?.GetType().FullName ?? "null"); - - dictionary.Remove(id); } }); return ValueTask.CompletedTask; @@ -59,8 +57,16 @@ public class WorkflowInstanceStorageDriver(IPayloadSerializer payloadSerializer, SerializerOptions = payloadSerializer.GetOptions() }; var result = node.TryConvertTo(variableType, options); - var parsedValue = result.IsSuccess ? result.Value : node; - return new (parsedValue); + if (result.IsSuccess) + return new(result.Value); + + logger.LogWarning(result.Exception, "Failed to convert stored variable '{VariableId}' to type '{VariableType}'. The stored value was left unread rather than returned as an untyped node.", + id, variableType.FullName); + + if (ObjectConverter.StrictMode) + result.ThrowIfFailure(); + + return new((object?)null); } /// diff --git a/test/unit/Elsa.Workflows.Core.UnitTests/VariableStorageDrivers/WorkflowInstanceStorageDriverTests.cs b/test/unit/Elsa.Workflows.Core.UnitTests/VariableStorageDrivers/WorkflowInstanceStorageDriverTests.cs new file mode 100644 index 000000000..b3e767fa0 --- /dev/null +++ b/test/unit/Elsa.Workflows.Core.UnitTests/VariableStorageDrivers/WorkflowInstanceStorageDriverTests.cs @@ -0,0 +1,137 @@ +using System.Text.Json; +using System.Text.Json.Nodes; +using Elsa.Expressions.Helpers; +using Elsa.Workflows.Memory; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; + +namespace Elsa.Workflows.Core.UnitTests.VariableStorageDrivers; + +[Collection(nameof(WorkflowInstanceStorageDriverTestsCollection))] +public class WorkflowInstanceStorageDriverTests +{ + [Fact] + public async Task WriteAsync_WhenSerializeFails_PreservesPreviousValue() + { + var harness = CreateHarness(new Variable("name", "kept")); + const string id = "nameVariable"; + + await harness.Driver.WriteAsync(id, "kept", harness.Context); + + var storedBefore = GetVariables(harness.Properties)[id].ToJsonString(); + + await harness.Driver.WriteAsync(id, CreateUnserializableValue(), harness.Context); + + var dictionary = GetVariables(harness.Properties); + Assert.True(dictionary.ContainsKey(id)); + Assert.Equal(storedBefore, dictionary[id].ToJsonString()); + + var read = await harness.Driver.ReadAsync(id, harness.Context); + Assert.Equal("kept", read); + } + + [Fact] + public async Task WriteAsync_WhenSerializeFailsWithNoPriorValue_DoesNotCreateEntry() + { + var harness = CreateHarness(new Variable("name", "")); + const string id = "nameVariable"; + + await harness.Driver.WriteAsync(id, CreateUnserializableValue(), harness.Context); + + Assert.False(GetVariables(harness.Properties).ContainsKey(id)); + } + + [Fact] + public async Task DeleteAsync_RemovesStoredValue() + { + var harness = CreateHarness(new Variable("name", "kept")); + const string id = "nameVariable"; + + await harness.Driver.WriteAsync(id, "kept", harness.Context); + await harness.Driver.DeleteAsync(id, harness.Context); + + Assert.False(GetVariables(harness.Properties).ContainsKey(id)); + } + + [Fact] + public async Task ReadAsync_WhenConvertFails_DoesNotReturnUntypedJsonNode() + { + var harness = CreateHarness(new Variable("count", 0)); + const string id = "countVariable"; + SeedIncompatibleNode(harness.Properties, id); + + var read = await harness.Driver.ReadAsync(id, harness.Context); + + Assert.Null(read); + Assert.False(read is JsonNode); + Assert.True(GetVariables(harness.Properties).ContainsKey(id)); + } + + [Fact] + public async Task ReadAsync_WhenConvertFailsAndStrictMode_Throws() + { + var harness = CreateHarness(new Variable("count", 0)); + const string id = "countVariable"; + SeedIncompatibleNode(harness.Properties, id); + + var originalStrictMode = ObjectConverter.StrictMode; + try + { + ObjectConverter.StrictMode = true; + + await Assert.ThrowsAnyAsync(() => harness.Driver.ReadAsync(id, harness.Context).AsTask()); + + Assert.True(GetVariables(harness.Properties).ContainsKey(id)); + } + finally + { + ObjectConverter.StrictMode = originalStrictMode; + } + } + + private static Harness CreateHarness(Variable variable) + { + var properties = new Dictionary(); + var executionContext = Substitute.For(); + executionContext.Properties.Returns(properties); + + var payloadSerializer = Substitute.For(); + payloadSerializer.GetOptions().Returns(new JsonSerializerOptions()); + + var driver = new WorkflowInstanceStorageDriver(payloadSerializer, NullLogger.Instance); + var context = new StorageDriverContext(executionContext, variable, CancellationToken.None); + + return new(driver, context, properties); + } + + private static VariablesDictionary GetVariables(IDictionary properties) => + (VariablesDictionary)properties[WorkflowInstanceStorageDriver.VariablesDictionaryStateKey]; + + private static void SeedIncompatibleNode(IDictionary properties, string id) + { + properties[WorkflowInstanceStorageDriver.VariablesDictionaryStateKey] = new VariablesDictionary + { + [id] = JsonNode.Parse("""{"foo":"bar"}""")! + }; + } + + private static CyclicValue CreateUnserializableValue() + { + var value = new CyclicValue(); + value.Self = value; + return value; + } + + private sealed record Harness( + WorkflowInstanceStorageDriver Driver, + StorageDriverContext Context, + IDictionary Properties); + + private sealed class CyclicValue + { + public CyclicValue Self { get; set; } = null!; + } +} + +[CollectionDefinition(nameof(WorkflowInstanceStorageDriverTestsCollection), DisableParallelization = true)] +public sealed class WorkflowInstanceStorageDriverTestsCollection;