Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Repro serializer issue #1116

Closed
wants to merge 9 commits into from
Closed

Repro serializer issue #1116

wants to merge 9 commits into from

Conversation

martincostello
Copy link
Owner

Changes to attempt to repro issue with polymorphism in .NET 9 preview 2.

#1039 (comment)

martincostello and others added 5 commits March 21, 2024 14:50
Update to preview 1 of .NET 9.
* Update .NET SDK

Update .NET SDK to version 9.0.100-preview.2.24157.14.

---
updated-dependencies:
- dependency-name: Microsoft.NET.Sdk
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: costellobot <102549341+costellobot@users.noreply.github.com>

* Bump .NET NuGet packages

Bumps .NET dependencies to their latest versions for the .NET 9.0.100-preview.2.24157.14 SDK.

Bumps Microsoft.AspNetCore.WebUtilities from 9.0.0-preview.1.24081.5 to 9.0.0-preview.2.24128.4.
Bumps Microsoft.Extensions.Configuration.Binder from 9.0.0-preview.1.24080.9 to 9.0.0-preview.2.24128.5.
Bumps Microsoft.Extensions.Configuration.EnvironmentVariables from 9.0.0-preview.1.24080.9 to 9.0.0-preview.2.24128.5.
Bumps Microsoft.Extensions.Configuration.Json from 9.0.0-preview.1.24080.9 to 9.0.0-preview.2.24128.5.
Bumps Microsoft.Extensions.DependencyInjection from 9.0.0-preview.1.24080.9 to 9.0.0-preview.2.24128.5.
Bumps Microsoft.Extensions.Http from 9.0.0-preview.1.24080.9 to 9.0.0-preview.2.24128.5.
Bumps Microsoft.Extensions.Http.Diagnostics from 9.0.0-preview.1.24108.1 to 9.0.0-preview.2.24157.4.
Bumps Microsoft.Extensions.Http.Resilience from 9.0.0-preview.1.24108.1 to 9.0.0-preview.2.24157.4.
Bumps Microsoft.Extensions.Logging.Console from 9.0.0-preview.1.24080.9 to 9.0.0-preview.2.24128.5.
Bumps Microsoft.Extensions.Telemetry from 9.0.0-preview.1.24108.1 to 9.0.0-preview.2.24157.4.
Bumps System.Text.Json from 9.0.0-preview.1.24080.9 to 9.0.0-preview.2.24128.5.

---
updated-dependencies:
- dependency-name: Microsoft.AspNetCore.WebUtilities
  dependency-type: direct:production
  update-type: version-update:semver-major
- dependency-name: Microsoft.Extensions.Configuration.Binder
  dependency-type: direct:production
  update-type: version-update:semver-major
- dependency-name: Microsoft.Extensions.Configuration.EnvironmentVariables
  dependency-type: direct:production
  update-type: version-update:semver-major
- dependency-name: Microsoft.Extensions.Configuration.Json
  dependency-type: direct:production
  update-type: version-update:semver-major
- dependency-name: Microsoft.Extensions.DependencyInjection
  dependency-type: direct:production
  update-type: version-update:semver-major
- dependency-name: Microsoft.Extensions.Http
  dependency-type: direct:production
  update-type: version-update:semver-major
- dependency-name: Microsoft.Extensions.Http.Diagnostics
  dependency-type: direct:production
  update-type: version-update:semver-major
- dependency-name: Microsoft.Extensions.Http.Resilience
  dependency-type: direct:production
  update-type: version-update:semver-major
- dependency-name: Microsoft.Extensions.Logging.Console
  dependency-type: direct:production
  update-type: version-update:semver-major
- dependency-name: Microsoft.Extensions.Telemetry
  dependency-type: direct:production
  update-type: version-update:semver-major
- dependency-name: System.Text.Json
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: costellobot <102549341+costellobot@users.noreply.github.com>

---------

Signed-off-by: costellobot <102549341+costellobot@users.noreply.github.com>
Use `AllowOutOfOrderMetadataProperties` to re-introduce polymorphsim.
Effectively reverts 83eb76f.
Add a deserialization test that uses `SourceGeneratorLambdaJsonSerializer`.
Add a serializer that logs the JSON before attempting to deserialize it.
Copy link

codecov bot commented Mar 23, 2024

Codecov Report

Attention: Patch coverage is 94.73684% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 92.85%. Comparing base (7fc2a65) to head (3545b96).

Files Patch % Lines
src/LondonTravel.Skill/Models/Request.cs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1116      +/-   ##
==========================================
- Coverage   92.97%   92.85%   -0.12%     
==========================================
  Files          45       49       +4     
  Lines         612      616       +4     
  Branches       62       59       -3     
==========================================
+ Hits          569      572       +3     
- Misses         23       24       +1     
  Partials       20       20              
Flag Coverage Δ
linux 92.85% <94.73%> (-0.12%) ⬇️
macos 92.85% <94.73%> (-0.12%) ⬇️
windows 92.85% <94.73%> (-0.12%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@martincostello
Copy link
Owner Author

/deploy dev

Copy link

github-actions bot commented Mar 23, 2024

🧪 Tests for deployment to dev failed ❌

Copy link

Payload Duration Billed Duration Memory Size Max Memory Used Init Duration Trace
Cancel 270.67 ms 656 ms 192 MB 70 MB 384.90 ms 🔗
UnknownIntent 27.50 ms 28 ms 192 MB 71 MB - 🔗
Status 89.92 ms 90 ms 192 MB 73 MB - 🔗
Disruption 16.48 ms 17 ms 192 MB 73 MB - 🔗
SessionEnded 27.20 ms 28 ms 192 MB 73 MB - 🔗
Stop 27.55 ms 28 ms 192 MB 73 MB - 🔗
Launch 26.86 ms 27 ms 192 MB 73 MB - 🔗
Help 26.84 ms 27 ms 192 MB 73 MB - 🔗

Set `LAMBDA_NET_SERIALIZER_DEBUG=true` to get the built-in logging too(?).
@martincostello
Copy link
Owner Author

/deploy dev

Copy link

github-actions bot commented Mar 23, 2024

🧪 Tests for deployment to dev failed ❌

Copy link

Payload Duration Billed Duration Memory Size Max Memory Used Init Duration Trace
Cancel 283.95 ms 669 ms 192 MB 70 MB 384.88 ms 🔗
UnknownIntent 26.96 ms 27 ms 192 MB 71 MB - 🔗
Status 87.13 ms 88 ms 192 MB 72 MB - 🔗
Disruption 28.10 ms 29 ms 192 MB 72 MB - 🔗
SessionEnded 27.51 ms 28 ms 192 MB 72 MB - 🔗
Stop 27.46 ms 28 ms 192 MB 73 MB - 🔗
Launch 27.11 ms 28 ms 192 MB 73 MB - 🔗
Help 26.25 ms 27 ms 192 MB 73 MB - 🔗

Specify `AllowOutOfOrderMetadataProperties=true` to customise the options the Lambda serializer uses.
@martincostello
Copy link
Owner Author

/deploy dev

Copy link

github-actions bot commented Mar 23, 2024

🧪 Tests for deployment to dev failed ❌

Copy link

Payload Duration Billed Duration Memory Size Max Memory Used Init Duration Trace
Cancel 1362.44 ms 1754 ms 192 MB 85 MB 391.52 ms 🔗
UnknownIntent 63.61 ms 64 ms 192 MB 85 MB - 🔗
Status 208.51 ms 209 ms 192 MB 88 MB - 🔗
Disruption 39.03 ms 40 ms 192 MB 88 MB - 🔗
SessionEnded 67.27 ms 68 ms 192 MB 88 MB - 🔗
Stop 27.83 ms 28 ms 192 MB 88 MB - 🔗
Launch 36.91 ms 37 ms 192 MB 89 MB - 🔗
Help 65.73 ms 66 ms 192 MB 89 MB - 🔗

Try to fix the JSON being split onto multiple lines.
@martincostello
Copy link
Owner Author

/deploy dev

Copy link

github-actions bot commented Mar 23, 2024

🧪 Tests for deployment to dev failed ❌

Copy link

Payload Duration Billed Duration Memory Size Max Memory Used Init Duration Trace
Cancel 1282.15 ms 1694 ms 192 MB 84 MB 411.48 ms 🔗
UnknownIntent 61.82 ms 62 ms 192 MB 85 MB - 🔗
Status 207.64 ms 208 ms 192 MB 87 MB - 🔗
Disruption 62.13 ms 63 ms 192 MB 87 MB - 🔗
SessionEnded 38.28 ms 39 ms 192 MB 87 MB - 🔗
Stop 18.22 ms 19 ms 192 MB 88 MB - -
Launch 26.82 ms 27 ms 192 MB 88 MB - 🔗
Help 86.13 ms 87 ms 192 MB 88 MB - 🔗

@martincostello
Copy link
Owner Author

martincostello commented Mar 23, 2024

Polymorphism issue resolved by customizing the AWS Lambda serializer to set AllowOutOfOrderMetadataProperties=true.

That's then exposed a new AoT-related issue:

{
    "EventId": 1,
    "LogLevel": "Error",
    "Category": "MartinCostello.LondonTravel.Skill.AlexaSkill",
    "Message": "Failed to handle request for session amzn1.echo-api.session.LAMBDA-TEST-DEV.",
    "Exception": "System.MissingMethodException: Constructor on type 'Polly.Registry.ResiliencePipelineRegistryOptions`1[Microsoft.Extensions.Http.Resilience.Internal.HttpKey]' not found.\n   at System.Activator.CreateInstance[T]() + 0x88\n   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String) + 0x38\n   at Microsoft.Extensions.Options.UnnamedOptionsManager`1.get_Value() + 0xbc\n   at Polly.PollyServiceCollectionExtensions.<>c__6`1.<AddResiliencePipelineRegistry>b__6_0(IServiceProvider serviceProvider) + 0x3c\n   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitFactory(FactoryCallSite, RuntimeResolverContext) + 0x1c\n   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitRootCache(ServiceCallSite, RuntimeResolverContext) + 0x74\n   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(ServiceCallSite, TArgument) + 0xb8\n   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.Resolve(ServiceCallSite, ServiceProviderEngineScope) + 0x34\n   at Microsoft.Extensions.DependencyInjection.ServiceProvider.CreateServiceAccessor(ServiceIdentifier) + 0x130\n   at Microsoft.Extensions.DependencyInjection.ServiceProvider.GetService(ServiceIdentifier, ServiceProviderEngineScope) + 0x384\n   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider, Type) + 0xc8\n   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider) + 0x30\n   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitFactory(FactoryCallSite, RuntimeResolverContext) + 0x1c\n   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitRootCache(ServiceCallSite, RuntimeResolverContext) + 0x74\n   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(ServiceCallSite, TArgument) + 0xb8\n   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.Resolve(ServiceCallSite, ServiceProviderEngineScope) + 0x34\n   at Microsoft.Extensions.DependencyInjection.ServiceProvider.CreateServiceAccessor(ServiceIdentifier) + 0x130\n   at Microsoft.Extensions.DependencyInjection.ServiceProvider.GetService(ServiceIdentifier, ServiceProviderEngineScope) + 0x384\n   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider, Type) + 0xc8\n   at Microsoft.Extensions.DependencyInjection.ResilienceHttpClientBuilderExtensions.CreatePipelineSelector(IServiceProvider, String) + 0x48\n   at Microsoft.Extensions.DependencyInjection.ResilienceHttpClientBuilderExtensions.<>c__DisplayClass6_0.<AddResilienceHandler>b__0(IServiceProvider serviceProvider) + 0x20\n   at Microsoft.Extensions.DependencyInjection.HttpClientBuilderExtensions.<>c__DisplayClass3_0.<AddHttpMessageHandler>b__1(HttpMessageHandlerBuilder) + 0x24\n   at Microsoft.Extensions.Http.DefaultHttpClientFactory.<>c__DisplayClass17_0.<CreateHandlerEntry>g__Configure|0(HttpMessageHandlerBuilder) + 0x78\n   at Microsoft.Extensions.Http.MetricsFactoryHttpMessageHandlerFilter.<>c__DisplayClass2_0.<Configure>b__0(HttpMessageHandlerBuilder) + 0x28\n   at Microsoft.Extensions.Http.LoggingHttpMessageHandlerBuilderFilter.<>c__DisplayClass6_0.<Configure>b__0(HttpMessageHandlerBuilder) + 0x30\n   at Microsoft.Extensions.Http.DefaultHttpClientFactory.CreateHandlerEntry(String) + 0x258\n   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode) + 0x50\n--- End of stack trace from previous location ---\n   at System.Lazy`1.CreateValue() + 0xdc\n   at Microsoft.Extensions.Http.DefaultHttpClientFactory.CreateHandler(String) + 0x58\n   at Microsoft.Extensions.Http.DefaultHttpClientFactory.CreateClient(String) + 0x2c\n   at Microsoft.Extensions.DependencyInjection.HttpClientBuilderExtensions.AddTransientHelper[TClient](IServiceProvider, IHttpClientBuilder) + 0x3c\n   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitFactory(FactoryCallSite, RuntimeResolverContext) + 0x1c\n   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitDisposeCache(ServiceCallSite, RuntimeResolverContext) + 0x20\n   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(ServiceCallSite, TArgument) + 0x154\n   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitConstructor(ConstructorCallSite, RuntimeResolverContext) + 0x88\n   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.VisitDisposeCache(ServiceCallSite, RuntimeResolverContext) + 0x20\n   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteVisitor`2.VisitCallSite(ServiceCallSite, TArgument) + 0x154\n   at Microsoft.Extensions.DependencyInjection.ServiceLookup.CallSiteRuntimeResolver.Resolve(ServiceCallSite, ServiceProviderEngineScope) + 0x34\n   at Microsoft.Extensions.DependencyInjection.ServiceProvider.GetService(ServiceIdentifier, ServiceProviderEngineScope) + 0x344\n   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider, Type) + 0xc8\n   at MartinCostello.LondonTravel.Skill.IntentFactory.Create(Intent) + 0x278\n   at MartinCostello.LondonTravel.Skill.AlexaSkill.<OnIntentAsync>d__5.MoveNext() + 0x38\n--- End of stack trace from previous location ---\n   at MartinCostello.LondonTravel.Skill.FunctionHandler.<HandleRequestAsync>d__4.MoveNext() + 0x198",
    "State": {
        "Message": "{OriginalFormat}=Failed to handle request for session {SessionId}.,SessionId=amzn1.echo-api.session.LAMBDA-TEST-DEV",
        "{OriginalFormat}": "Failed to handle request for session {SessionId}.",
        "SessionId": "amzn1.echo-api.session.LAMBDA-TEST-DEV"
    }
}

Remove the payload logging.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants