aws-lambda-dotnet icon indicating copy to clipboard operation
aws-lambda-dotnet copied to clipboard

Amazon.Lambda.CloudWatchEvents definition doesn't work with Newtonsoft.Json when compiling on .NET Core 3.1

Open bjorg opened this issue 5 years ago • 8 comments

For .NET Core 3.1 compilation target, Amazon.Lambda.CloudWatchEvents automatically assumes that the function uses System.Text.Json. However, if it uses Newtonsoft.Json like before, because only the target framework was changed, then the DetailType property is not properly deserialized.

The problematic line is here: https://github.com/aws/aws-lambda-dotnet/blob/master/Libraries/src/Amazon.Lambda.CloudWatchEvents/CloudWatchEvent.cs#L43

bjorg avatar Apr 18 '20 00:04 bjorg

I'm not sure why you would see any change in behavior when targeting .NET Core 3.1. There wasn't any special logic for DetailType and the unit test that tests both serializer is passing with a value for that field. Can you include a sample of the event you are seeing?

https://github.com/aws/aws-lambda-dotnet/blob/master/Libraries/test/EventsTests.Shared/EventTests.cs#L1249-L1266

normj avatar Apr 18 '20 06:04 normj

Maybe I jumped the gun. Here's what I saw in the CloudWatch logs when I emitted the received event. Notice how DetailType is null.

{
    "Version": "0",
    "Account": "xyz",
    "Region": "us-west-2",
    "Detail": {
        "Message": "hello world!"
    },
    "DetailType": null,
    "Source": "LambdaSharp.Sample",
    "Time": "2020-04-18T01:15:39Z",
    "Id": "ab26e799-0b3b-637c-2f96-6a428401fdf3",
    "Resources": [
        "lambdasharp:stack:SteveBvNext-Sample-Event",
        "lambdasharp:module:Sample.Event",
        "lambdasharp:tier:SteveBvNext"
    ]
}

This is the actual event that was sent:

{
  "version": "0",
  "id": "ab26e799-0b3b-637c-2f96-6a428401fdf3",
  "detail-type": "MyFirstEvent",
  "source": "LambdaSharp.Sample",
  "account": "xyz",
  "time": "2020-04-18T01:15:39Z",
  "region": "us-west-2",
  "resources": [
    "lambdasharp:stack:SteveBvNext-Sample-Event",
    "lambdasharp:module:Sample.Event",
    "lambdasharp:tier:SteveBvNext"
  ],
  "detail": {
    "Message": "hello world!"
  }
}

This is the code I used, following the pre-3.1 pattern:

[assembly: LambdaSerializer(typeof(Amazon.Lambda.Serialization.Json.JsonSerializer))]

namespace Sample.Event.ReceiverFunction {

    public class EventDetails {

        //--- Properties ---
        public string? Message { get; set; }
    }

    public class FunctionResponse { }

    public class Function : ALambdaFunction<CloudWatchEvent<EventDetails>, FunctionResponse> {

        //--- Methods ---
        public override async Task InitializeAsync(LambdaConfig config) { }

        public override async Task<FunctionResponse> ProcessMessageAsync(CloudWatchEvent<EventDetails> request) {
            LogInfo(SerializeJson(request));
            return new FunctionResponse();
        }
    }
}

For context, SerializeJson uses the Amazon.Lambda.Serialization.Json.JsonSerializer.

Full code: https://github.com/LambdaSharp/LambdaSharpTool/blob/WIP-v0.7-DotNetCore31/Samples/EventSample/ReceiverFunction/Function.cs

It's certainly possible that the issue was present before 3.1. I didn't test it until now.

bjorg avatar Apr 18 '20 06:04 bjorg

I think I figured it out!

The problem stems from this line line: https://github.com/aws/aws-lambda-dotnet/blob/c7cb94721a20efdec9cc98aac7e899848d1f415f/Libraries/src/Amazon.Lambda.Serialization.Json/AwsResolver.cs#L127

The code requires that the serialized type MUST belong to the Amazon.Lambda.CloudWatchEvents namespace. I was able to confirm this by creating two version of the same function, which should behave identically, but don't.

Buggy Version

In the first version, I use CloudWatchEvent<T> directly and it does not work. DetailType is logged as an empty string (null).

namespace CloudWatchEventListener {

    public class EventDetails {

        //--- Properties ---
        public string Message { get; set; }
    }

    public class Function {

        //--- Methods ---
        public string FunctionHandler(CloudWatchEvent<EventDetails> request, ILambdaContext context) {
            Console.WriteLine($"DetailType = {request.DetailType}"); // <-- DetailType is empty
            return "Okay";
        }
    }
}

Fixed Version

In this version, I created a derived type in the Amazon.Lambda.CloudWatchEvents namespace and it worked as expected.

namespace Amazon.Lambda.CloudWatchEvents {

    public class MyCloudWatchEvent : CloudWatchEvent<CloudWatchEventListener.EventDetails> { }
}

namespace CloudWatchEventListener {

    public class EventDetails {

        //--- Properties ---
        public string Message { get; set; }
    }

    public class Function {

        //--- Methods ---
        public string FunctionHandler(MyCloudWatchEvent request, ILambdaContext context) {
            Console.WriteLine($"DetailType = {request.DetailType}"); // <-- DetailType is shown
            return "Okay";
        }
    }
}

I created a repository with both function version. Hopefully that helps in reproducing and fixing this issue. Assuming it's still relevant since it only applies to Newtonsoft.Json serialization. https://github.com/bjorg/CloudWatchEventSerializationBug

bjorg avatar Apr 29 '20 16:04 bjorg

Any hope of getting this fixed eventually?

bjorg avatar Oct 14 '20 22:10 bjorg

Probable fix is to remove condition type.FullName.StartsWith("Amazon.Lambda.CloudWatchEvents.") in the code https://github.com/aws/aws-lambda-dotnet/blob/c7cb94721a20efdec9cc98aac7e899848d1f415f/Libraries/src/Amazon.Lambda.Serialization.Json/AwsResolver.cs#L127

Needs review from development team.

ashishdhingra avatar Feb 04 '21 19:02 ashishdhingra

We have noticed this issue has not received attention in 1 year. We will close this issue for now. If you think this is in error, please feel free to comment and reopen the issue.

github-actions[bot] avatar Feb 05 '22 00:02 github-actions[bot]

Do not close.

ashishdhingra avatar Feb 05 '22 00:02 ashishdhingra

This is causing me an issue right now as well. The offending line is in fact the second part of the condition mentioned in https://github.com/aws/aws-lambda-dotnet/issues/634#issuecomment-773545038 since the base type of, for instance, CloudWatchEvent<JObject>, is just Object, not CloudWatchEvent<>. It's just totally faulty logic. The only way I've been able to get around it is by making my own class and hacking it into the AWS namespace as follows:

namespace Amazon.Lambda.CloudWatchEvents
{
    public class MyHackyCloudWatchEvent<T> : CloudWatchEvent<T>
    {
    }
}

which is, obviously, fairly weird.

I think what the author was intending to do was check that the current type is a reification of an open generic type CloudWatchEvent<T>, which would entail calling type.GetGenericTypeDefinition() and checking if that is typeof(CloudWatchEvent<>).

jamesbascle avatar Jun 09 '22 21:06 jamesbascle