vivet / GoogleApi

C# .NET Core Google Api (Maps, Places, Roads, Search, Translate). Supports all endpoints and requests / responses.
MIT License
544 stars 153 forks source link

NullReferenceException in UWP app with .NET Native, starting with GoogleApi 4.3.0 #326

Closed mfeingol closed 1 year ago

mfeingol commented 1 year ago

I'm leaving this here in case it helps someone else. Also because @vivet if you're so inclined I think you can fix it.

After upgrading from GoogleApi 4.2.6 to higher versions, I began seeing a NullReferenceException in my UWP app on startup, but only when it was compiled in release mode with .NET Native. The stack trace was the following:

    System.Text.Json.dll!System.Text.Json.Reflection.ReflectionExtensions.CreateInstanceNoWrapExceptions(System.Type type, System.Type[] parameterTypes, object[] parameters)   Unknown
    System.Text.Json.dll!System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.CreateJsonTypeInfo(System.Type type, System.Text.Json.JsonSerializerOptions options)   Unknown
    System.Text.Json.dll!System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.GetTypeInfo(System.Type type, System.Text.Json.JsonSerializerOptions options)  Unknown
    System.Text.Json.dll!System.Text.Json.JsonSerializerOptions.GetTypeInfoNoCaching(System.Type type)  Unknown
    Microsoft.EntityFrameworkCore.Relational.dll!Microsoft.EntityFrameworkCore.Update.Internal.SharedTableEntryMapFactory<System.__Canon>.Invoke(Microsoft.EntityFrameworkCore.Update.Internal.SharedTableEntryValueFactory<System.__Canon> valueFactory)   Unknown
    System.Collections.Concurrent.dll!System.Collections.Concurrent.ConcurrentDictionary<System.Type, System.Text.Json.Serialization.Metadata.JsonTypeInfo>.GetOrAdd(System.Type key, System.Func<System.Type, System.Text.Json.Serialization.Metadata.JsonTypeInfo> valueFactory)  Unknown
    System.Text.Json.dll!System.Text.Json.JsonSerializerOptions.CachingContext.GetOrAddJsonTypeInfo(System.Type type)   Unknown
    System.Text.Json.dll!System.Text.Json.JsonSerializerOptions.GetTypeInfoInternal(System.Type type, bool ensureConfigured, bool resolveIfMutable) Unknown
    System.Text.Json.dll!System.Text.Json.Serialization.Metadata.JsonPropertyInfo.Configure()   Unknown
    System.Text.Json.dll!System.Text.Json.Serialization.Metadata.JsonPropertyInfo.EnsureConfigured()    Unknown
    System.Text.Json.dll!System.Text.Json.Serialization.Metadata.JsonTypeInfo.InitializePropertyCache() Unknown
    System.Text.Json.dll!System.Text.Json.Serialization.Metadata.JsonTypeInfo.Configure()   Unknown
    System.Text.Json.dll!System.Text.Json.Serialization.Metadata.JsonTypeInfo.EnsureConfigured.__ConfigureLocked|143_0()    Unknown
    System.Text.Json.dll!System.Text.Json.JsonSerializerOptions.GetTypeInfoInternal(System.Type type, bool ensureConfigured, bool resolveIfMutable) Unknown
    System.Text.Json.dll!System.Text.Json.JsonSerializer.GetTypeInfo(System.Text.Json.JsonSerializerOptions options, System.Type inputType) Unknown
    System.Text.Json.dll!System.Text.Json.JsonSerializer.DeserializeAsync<System.__Canon>(System.IO.Stream utf8Json, System.Text.Json.JsonSerializerOptions options, System.Threading.CancellationToken cancellationToken)  Unknown
    Microsoft.Graph.Core.dll!Microsoft.Graph.Serializer.DeserializeObject<System.__Canon>(System.IO.Stream stream)  Unknown
    Microsoft.Graph.Core.dll!Microsoft.Graph.ResponseHandler.HandleResponse<System.__Canon>(System.Net.Http.HttpResponseMessage response)   Unknown
    Microsoft.Graph.Core.dll!Microsoft.Graph.BaseRequest.SendAsync<System.__Canon>(object serializableObject, System.Threading.CancellationToken cancellationToken, System.Net.Http.HttpCompletionOption completionOption)  Unknown

The root cause turned out to be GoogleApi switching from Newtonsoft to System.Text.Json in 4.3.0. Adding an entry like this to my app's Default.rd.xml manifest file solved the problem:

<Assembly Name="System.Text.Json" Dynamic="Required PublicAndInternal" />

I believe a similar manifest entry can be created in the GoogleApi assembly so nobody has to do this manually. cf https://stackoverflow.com/questions/39687011/how-to-properly-include-an-rd-xml-file-in-a-nuget-package

vivet commented 1 year ago

Hi I will take a look at it

vivet commented 1 year ago

Hi

I took a look at the stackoverflow post. I am a bit unsure what exactly needs to be done, when there is no *.rd.xml files.

Should I add the dynamic part to the NuGet rerference?, like this: <PackageReference Include="System.Text.Json" Version="7.0.0" Dynamic="Required PublicAndInternal" />

Maybe you can create a PR, showing how you think it should be solved?

vivet commented 1 year ago

Are you in some way using the models from GoogleApi with graph or entity framework? When i looked a bit closer on the exception, it doesn't seem to originate from GoogleApi, but from those...

mfeingol commented 1 year ago

cf https://devblogs.microsoft.com/dotnet/net-native-deep-dive-making-your-library-great/ for more clues on how to embed a Library.rd.xml file. I've never done it myself, but from what I can tell you just add the XML file as an embedded resource inside your assembly.

Fwiw, I ended up with the following directives:

        <Assembly Name="GoogleApi" Dynamic="Required All" />
        <Assembly Name="System.Text.Json" Dynamic="Required PublicAndInternal" />

It's possible they can be scoped down further for greater precision, but I ran out of time.

The first fixes an exception when geocoding (and I assume other places) where serialization fails due to missing default constructors. The second addresses the NullReferenceException I mentioned above.

As for why the NRE was happening... I have no idea. I think the EF stack frame there is a red herring, since the issue occurs when I attempt to use Graph to auth and read from OneDrive. Why this only happens when GoogleApi 4.3.0 or above is used is a mystery to me. But I confirmed it repeatedly.

If you'd like to play around with my project, I can give you access to it.

vivet commented 1 year ago

Before we dig any deeper i need to see the full exception, showing that this is caused by GoogleApi. From the exception you pasted initially, i can't determine that this is GoogleApi, and i rather not change something i can't prove correctly.

It really seems like you are using the models for something afterwards with graph api.

mfeingol commented 1 year ago

There's no relationship at all between what Graph is doing during its request and GoogleApi. All I can think of is System.Text.Json is doing something really weird, like scanning all assemblies in the process for relevant attributes, or some such. But I can tell you 100% that this repros when I depend on GoogleApi 4.3.0 and above, and 100% not with 4.2.6.

It took me quite a while to track that down, for precisely the same reasons you're dubious. :-)

Anyway, let me know if you want to run my app to see for yourself. You'll need a Windows machine, needless to say.

mfeingol commented 1 year ago

I should note that I could see you deciding the System.Text.Json issue is someone else's problem (e.g. System.Text.Json itself, or the app's) despite the weird circumstances. But you might want to add the other directive for your own assembly in order to avoid .NET Native issues with reflection on your own types.

vivet commented 1 year ago

Okay it's good that the models are not used for graph or similar. I understand that the problem started from v4.3.0, where I switched to System.Text.Json, but I still need to be sure this should be handled by GoogleApi and not the applications consuming it.

I don't need to run the app myself, or at least I am not sure it will gain us anything, cause I am pure backend developer, so my skills might not be sufficient to understand your app. Also, I haven't decided whether this is GoogleApi or "someone else's problem" - still gathering information and investigating.

It would be nice if you can supply the full exception stacktrace.

Have you tried building the source with your proposed changes and see if it works?

vivet commented 1 year ago

I read through the Microsoft blog post.

Would this solve the problem?

GoogleApi.rd.xml (embedded)

<?xml version="1.0" encoding="utf-8"?>
 <Directives xmlns="http://schemas.microsoft.com/netfx/2013/01/metadata">
   <Library Name="MyLibrary">
     <Assembly Name="System.Text.Json" Dynamic="Required PublicAndInternal" />
   </Library>
 </Directives>
mfeingol commented 1 year ago

Also, I haven't decided whether this is GoogleApi or "someone else's problem" - still gathering information and investigating.

Yeah, the System.Text.Json-related issue is pretty weird and I agree I'm not 100% sure it's your problem to solve. The weirdest thing is I have other indirect dependencies on System.Text.Json, but yours was the one that triggered the issue. And fwiw I tried adding the assembly directive just for GoogleApi, but it didn't help with that problem.

It would be nice if you can supply the full exception stacktrace.

For which of the two issues? The System.Text.Json stacktrace is in the original post. The second stack trace I don't have on hand, but could repro for you if necessary. It's nothing surprising, just a call to GoogleApi.GoogleMaps.Geocode.LocationGeocode.QueryAsync where deserialization fails due to the lack of a default constructor (optimized away by .NET Native).

Would this solve the problem?

I'm thinking you might want to go with adding a directive just for GoogleApi until we understand the System.Text.Json issue better. But it's your call.

I don't need to run the app myself

I was suggesting it mainly for you to be able to reproduce the issue and see the stack trace for yourself. But you may be able to do that with a simpler UWP app as well.

vivet commented 1 year ago

If you figure it out, I will gladly update the library to support this

mfeingol commented 1 year ago

The directive for GoogleApi is needed. The other one we should probably punt on.

vivet commented 1 year ago

Okay, are you able to make a PR?

vivet commented 1 year ago

Since, I haven't heard anything back, I will close the issue.

Note, that it seems that UWP is on a fast-track to deprecation, so consider upgrading to the newer "Windows App SDK" instead.

mfeingol commented 1 year ago

Sorry, I've been busy. That outcome is probably fair.

mfeingol commented 1 year ago

And btw, WinUI 3 doesn't have a MapControl, so not all apps can be upgraded.

vivet commented 1 year ago

Okay, I am no expert on Windows apps 😃

Feel free to reopen the issue or submit a PR in the future, if you figure out a clear path for how it shiuld be implemented.

vivet commented 1 year ago

I have reopened this issue. I am not sure how to fix it, as I am not familiar with UWP, but maybe someone do.

vivet commented 1 year ago

I have read a bit more about the subject, and created the rd.xml file. I have pushed a branch: https://github.com/vivet/GoogleApi/tree/ISSUE_326

Could you verify if it's working for you?

vivet commented 1 year ago

Stale issue. Will close.