Skip to content

[release/8.0-preview4] Add suppression message to JSON serialization #47910

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

Merged

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Apr 26, 2023

Backport of #47908 to release/8.0-preview4

/cc @eerhardt

Add suppression message to JSON serialization

Adds trimming and AOT suppressions in the Minimal API source generated code

Description

With the latest aspnetcore and Minimal API Source Generator, Native AOT users are getting a warning coming from generated code:

Trim analysis error IL2026: Microsoft.AspNetCore.Http.Generated.<GeneratedRouteBuilderExtensions_g>F69328E0708B4B584C5AACA22FE2C51A1CF192D6622828F613FC57C583CA77B63__GeneratedRouteBuilderExtensionsCore.ExecuteObjectResult(Object, HttpContext): Using member 'Microsoft.AspNetCore.Http.HttpResponseJsonExtensions.WriteAsJsonAsync<TValue>(HttpResponse, TValue, CancellationToken)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved.

The issue that this warning is alerting us to is handled through the JsonSerializer.IsReflectionEnabledByDefault feature switch and can be suppressed.

Customer Impact

With Native AOT (and trimming), the warnings are the main communication to the app developer that their app might not work after publishing. If an app doesn't contain warnings, we guarantee the app will continue to work as it did before publishing.

By always having an errant warning coming from generated code, developers will be confused and think they are doing something wrong. This makes the AOT experience unacceptable. The warnings need to be actionable by the developer.

Regression?

  • Yes
  • No

8.0-preview3 didn't have any AOT warnings in the dotnet new api --aot template. Without this change, it does now.

Risk

  • High
  • Medium
  • Low

The change is just adding a warning suppression to the Minimal API source generated code. There is already the same suppression in another place. Adding it in one more is very unlikely to cause problems.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

This was missed in #47859 and is causing trimming/AOT warnings from generated code.
@ghost ghost added the area-runtime label Apr 26, 2023
@eerhardt eerhardt added the Servicing-consider Shiproom approval is required for the issue label Apr 26, 2023
@ghost
Copy link

ghost commented Apr 26, 2023

Hi @github-actions[bot]. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@eerhardt eerhardt added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Apr 27, 2023
@ghost
Copy link

ghost commented Apr 27, 2023

Hi @github-actions[bot]. This PR was just approved to be included in the upcoming servicing release. Somebody from the @dotnet/aspnet-build team will get it merged when the branches are open. Until then, please make sure all the CI checks pass and the PR is reviewed.

@eerhardt
Copy link
Member

Approved in email.

@wtgodbe wtgodbe merged commit b6500ea into release/8.0-preview4 Apr 27, 2023
@wtgodbe wtgodbe deleted the backport/pr-47908-to-release/8.0-preview4 branch April 27, 2023 16:35
@ghost ghost added this to the 8.0-preview4 milestone Apr 27, 2023
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Apr 27, 2023
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants