tmenier / Flurl

Fluent URL builder and testable HTTP client for .NET
https://flurl.dev
MIT License
4.23k stars 387 forks source link

Avoid loading System.Text.Json when it isn't used #845

Open cremor opened 1 month ago

cremor commented 1 month ago

I have a .NET Framework client application that uses Flurl.Http. After upgrading to Flurl v4 this application now tries to load the System.Text.Json assembly, even though I use Flurl.Http.Newtonsoft.

Please make it so that System.Text.Json is not needed if Flurl.Http.Newtonsoft is used. Otherwise I'd have to deploy System.Text.Json assemblies (and the whole dependency tree that comes with it, which is huge!) to my clients even though it wouldn't (shouldn't) be used.

Sample (compile with <TargetFramework>net48</TargetFramework>):

static async Task Main()
{
    // Place a breakpoint here and delete all System.* assemblies in the bin folder before continuing.
    FlurlHttp.Clients.UseNewtonsoft();

    // Use the debugger to analyze the exception because 'ex.ToString()' also throws an exception itself for some reason...
    var result = await "https://some-api.com".GetStringAsync();
}

Exception:

System.TypeInitializationException: 'The type initializer for 'Flurl.Http.Configuration.FlurlHttpSettings' threw an exception.'
   at Flurl.Http.Configuration.FlurlHttpSettings.<<Get>g__prioritize|26_0>d`1.MoveNext()
   at Flurl.Http.Configuration.FlurlHttpSettings.Get[T](String propName)
   at Flurl.Http.Configuration.FlurlHttpSettings.get_HttpVersion()
   at Flurl.Http.FlurlClient.<SendAsync>d__23.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Flurl.Http.ResponseExtensions.<ReceiveString>d__1.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()
   at ConsoleApp2.Program.<Main>d__0.MoveNext() in C:\Daten\Projekte\ConsoleApp2\ConsoleApp2\Program.cs:line 18

Inner exception:
System.IO.FileNotFoundException: 'Could not load file or assembly 'System.Text.Json, Version=6.0.0.4, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51' or one of its dependencies.'
   at Flurl.Http.Configuration.DefaultJsonSerializer..ctor(JsonSerializerOptions options)
   at Flurl.Http.Configuration.FlurlHttpSettings..cctor()
tmenier commented 1 month ago

I honestly don't think what you're asking for is possible. Opting in to Newtonsoft happens at runtime so it can't be known at compile time. I understand the footprint is a problem, so I think this is a rare case where I would advise against upgrading to 4.0. 3.x is stable, and using a legacy version of Flurl on legacy platforms seems reasonable.

cremor commented 1 month ago

The compile time reference shouldn't be a problem. I use other packages with a compile time reference to STJ and they don't cause problems.

It's the fact that it's actually called at runtime when the default settings are created. Would it be possible to special case the default serializer setting? Maybe by making it Lazy<T>? Because when it is used the Newtonsoft one should already be set.

tmenier commented 1 month ago

I use other packages with a compile time reference to STJ and they don't cause problems.

Which ones and what problems do they not cause? The way I understand it, the entire footprint of STJ will be baked into your app because it's baked into Flurl. The difference is STJ won't be loaded into memory if it's not invoked. Is that all you need? If so, I agree, this should be possible with only small changes.

cremor commented 1 month ago

I'd have to check tomorrow to list all such packages. But I remember one right now: Oracle.ManagedDataAccess. I'm using version 19.x of that package and that only introduced the STJ reference a few months ago. There was a thread in the Oracle Forums in which an Oracle employee confirmed that you don't have to ship STJ if you don't use the JSON features of the client (basically if you don't use JSON columns).

Yes, all I need is that the STJ assembly is not loaded at runtime. I don't care about having it in my local dev environment and I don't care about .NET packaging (because my app is distributed with a custom installer that manually defines all dependencies).

tmenier commented 1 month ago

Ok, so to summarize you're concerned with memory consumption (due to runtime assembly loading) and not package footprint? I agree that that may be solvable with a smallish modification.

cremor commented 1 month ago

No, my main concern is packaging.

But the reduced memory footprint is a nice bonus.

tmenier commented 1 month ago

Ok, I'm a lot less familiar with how installers work than you are (and I'm not looking to get educated on it to be honest), so bear with me. You're not asking for any change to how Flurl.Http references its dependencies, correct? You only need the instantiation of the default JSON serializer to happen lazily, i.e. not at all when Newtonsoft is used? I can't speak to whether or not that will get you the outcome you're looking for, but if this is all you need, I can do that.

cremor commented 1 month ago

Yes, as far as I know that is all that I ask for.

In theory there could be additional runtime usages of STJ, but my sample should be able to show that after the change in FlurlHttpSettings.

edit: If you could provide a preview package for this change then I can test it with my actual application. Just in case the simple sample doesn't catch all.