Azure / azure-functions-durable-extension

Durable Task Framework extension for Azure Functions
MIT License
717 stars 271 forks source link

Microsoft.Azure.Functions.Worker.Extensions.DurableTask 1.1.1 Serialization Error #2767

Closed RichardBurns1982 closed 8 months ago

RichardBurns1982 commented 8 months ago

Description

We cannot upgrade to Microsoft.Azure.Functions.Worker.Extensions.DurableTask 1.1.1 as serialization of objects is broken when calling activitiies. If we downgrade to Microsoft.Azure.Functions.Worker.Extensions.DurableTask 1.1.0 everything works again.

Expected behavior

When passing through complex objects they should serialize without issue.

Actual behavior

We receive the following error on 1.1.1:

System.Text.Json.JsonException: The JSON value could not be converted to System.String. Path: $ | LineNumber: 0 | BytePositionInLine: 1. ---> System.InvalidOperationException: Cannot get the value of a token type 'StartObject' as a string. at System.Text.Json.ThrowHelper.ThrowInvalidOperationExceptionExpectedString(JsonTokenType tokenType) at System.Text.Json.Utf8JsonReader.GetString() at System.Text.Json.Serialization.JsonConverter1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value) at System.Text.Json.Serialization.JsonConverter1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state) --- End of inner exception stack trace --- at System.Text.Json.ThrowHelper.ReThrowWithPath(ReadStack& state, Utf8JsonReader& reader, Exception ex) at System.Text.Json.Serialization.JsonConverter1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state) at System.Text.Json.Serialization.JsonConverter1.ReadCoreAsObject(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state) at System.Text.Json.JsonSerializer.ReadFromSpan[TValue](ReadOnlySpan1 utf8Json, JsonTypeInfo jsonTypeInfo, Nullable1 actualByteCount) at System.Text.Json.JsonSerializer.Deserialize(ReadOnlySpan`1 utf8Json, Type returnType, JsonSerializerOptions options) at Azure.Core.Serialization.JsonObjectSerializer.Deserialize(Stream stream, Type returnType, CancellationToken cancellationToken) at Microsoft.Azure.Functions.Worker.Extensions.DurableTask.ObjectConverterShim.Deserialize(String data, Type targetType) in //src/Worker.Extensions.DurableTask/ObjectConverterShim.cs:line 32 at Microsoft.Azure.Functions.Worker.Extensions.DurableTask.ActivityInputConverter.ConvertAsync(ConverterContext context) in /_/src/Worker.Extensions.DurableTask/ActivityInputConverter.cs:line 38 at Microsoft.Azure.Functions.Worker.Context.Features.DefaultInputConversionFeature.ConvertAsyncUsingConverter(IInputConverter converter, ConverterContext context) in D:\a_work\1\s\src\DotNetWorker.Core\Context\Features\DefaultInputConversionFeature.cs:line 91 at Microsoft.Azure.Functions.Worker.Context.Features.DefaultInputConversionFeature.ConvertAsync(ConverterContext converterContext) in D:\a_work\1\s\src\DotNetWorker.Core\Context\Features\DefaultInputConversionFeature.cs:line 60 at Microsoft.Azure.Functions.Worker.Context.Features.DefaultFunctionInputBindingFeature.ConvertAsync(FunctionContext context, FunctionParameter parameter, IConverterContextFactory converterContextFactory, IInputConversionFeature inputConversionFeature, Object source) in D:\a_work\1\s\src\DotNetWorker.Core\Context\Features\DefaultFunctionInputBindingFeature.cs:line 157 at Microsoft.Azure.Functions.Worker.Context.Features.DefaultFunctionInputBindingFeature.BindFunctionInputAsync(FunctionContext context) in D:\a_work\1\s\src\DotNetWorker.Core\Context\Features\DefaultFunctionInputBindingFeature.cs:line 55 at FunctionAppOrchestrationSandbox.DirectFunctionExecutor.ExecuteAsync(FunctionContext context) in D:\GitHub\isolated-DurableTask-1.1.1-serialization-error\FunctionAppOrchestrationSandbox\Microsoft.Azure.Functions.Worker.Sdk.Generators\Microsoft.Azure.Functions.Worker.Sdk.Generators.FunctionExecutorGenerator\GeneratedFunctionExecutor.g.cs:line 31 at Microsoft.Azure.Functions.Worker.OutputBindings.OutputBindingsMiddleware.Invoke(FunctionContext context, FunctionExecutionDelegate next) in D:\a_work\1\s\src\DotNetWorker.Core\OutputBindings\OutputBindingsMiddleware.cs:line 13 at Microsoft.Azure.Functions.Worker.FunctionsApplication.InvokeFunctionAsync(FunctionContext context) in D:\a_work\1\s\src\DotNetWorker.Core\FunctionsApplication.cs:line 77 Microsoft.Azure.Functions.Worker.FunctionsApplication: Error: Function 'ExampleActivity', Invocation id 'a73fd25a-fff8-457f-aee4-f0a9f65c147e': An exception was thrown by the invocation.

Relevant source code snippets

Example repo: https://github.com/RichardBurns1982/isolated-DurableTask-1.1.1-serialization-error/blob/main/FunctionAppOrchestrationSandbox/ExampleOrchestrationFunction.cs

If you change Microsoft.Azure.Functions.Worker.Extensions.DurableTask to 1.1.0 it works, if you change to Microsoft.Azure.Functions.Worker.Extensions.DurableTask 1.1.1 you receive the json error above.

Known workarounds

Downgrade to Microsoft.Azure.Functions.Worker.Extensions.DurableTask 1.1.0

App Details

facundoam commented 8 months ago

Had the same issue, with a function using a primitive string as input variable...Had to fix it changing that to be a JsonObject instead, and then using System.Text instead of JsonConvert...

This all started happening when we updated all packages as well when migrating from net 6 to 8.

From this: public async Task Run([ActivityTrigger] string input) to this: public async Task Run([ActivityTrigger] JsonObject input)

and

From this: var data = JsonConvert.DeserializeObject(input)!; to this: var data = input.Deserialize()!;

davidmrdavid commented 8 months ago

Hi @RichardBurns1982.

I took a look at your repro, and want to confirm a few things.

In your repro, you have an orchestrator calling an activity as such:

// in orchestrator
           var message = new ParentMessage
           {
               Message = "Parent message",
               Child = new ChildMessage
               {
                   Message = "Child message"
               }
           };

           var nextPageNumber = await context.CallActivityAsync<int?>(
                   nameof(ExampleActivity),
                   message
               );

so the type of the activity input is ParentMessage.

Yet, in the Activity, the input is declared as string, as shown below:

        [Function(nameof(ExampleActivity))]
        public Task ExampleActivity(
            [ActivityTrigger] string message, 
            CancellationToken cancellationToken = default)

So that would explain to me why you have a serialization error: at the orchestrator, we declare the input as an Object, but at the receiver side/the activity, we demand a string, and there lies the mismatch.

If we change the Activity to be written with the expectation that the ParentMessage object will be received, then the code works for me. Here's the Activity that works on my end:

        [Function(nameof(ExampleActivity))]
        public Task ExampleActivity(
            [ActivityTrigger] ParentMessage message,
            CancellationToken cancellationToken = default)
        {
            //var parentMessage = JsonNode.Parse(message);
            //var childMessage = JsonNode.Parse(parentMessage[nameof(ParentMessage.Child)].ToString());
            //var childMessageType = childMessage[nameof(BaseMessage.MessageType)].ToString();

            switch (message.Child.MessageType)
            {
                case MessageTypeEnum.Child:
                    ChildMessage child = (ChildMessage)message.Child;
                    _logger.LogInformation($"Processing child message {child.Message}.");
                    break;
            }

            _logger.LogInformation($"Processing message {message}.");
            return Task.CompletedTask;
        }

Does that make sense? Not sure if before you were relying on buggy behavior (now fixed) that forced you to receive a string in the Activity. Can you tell me a bit more about why you declared the input as a string and not a ParentMessage? I want to make sure I'm not missing something here. Thanks!

jviau commented 8 months ago

@RichardBurns1982 this was intentioned changed in 1.1.1 as the previous behavior was bug in relation to in-proc behavior. What you are trying to do with the string input for an activity is essentially ask DF to not deserialize the input at all (instead give the raw input). We can evaluate adding this behavior separately, but we cannot do it via string type because we cannot differentiate between asking for raw input vs a string was the actual provided input type to the activity.

The problem stems from how passing a string input to an activity works - it will be serialized, and we are subject to whatever the configured serialize and deserialize behavior is. In the case of STJ (the default), passing a string input will wrap it in escaped quotes. So, to get back to the originally supplied string, we need to deserialize on the other end.

In short:

facundoam commented 8 months ago

As @davidmrdavid pointed out: in your scenario, you are parsing to a JsonNode right away. Instead of string, you could declare it as JsonObject or better yet ParentMessage and we will use STJ to deserialize the input into that type.

As stated, using JsonObject and STJ indeed, solves the issue. We had to update all our string references to be JsonObjects

RichardBurns1982 commented 8 months ago

Thank you both for your response, I understand the change now.

You are correct this is an old function from the in-proc days we haven't changed since moving to isolated. In our real world scenario there are many types of object inhering from BaseMessage which are on the parent object. We're trying to handle polymorphic deserialization since moving from newtonsoft to Text library so we aren't sure what the type of ChildMessage is.

We've rewritten this function now to remove the need to throw a string around and we are taking a JsonNode and all is working.

Thanks again for the responses, I'll close this one off.