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

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

Open
cremor opened this issue Oct 15, 2024 · 8 comments
Open

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

cremor opened this issue Oct 15, 2024 · 8 comments

Comments

@cremor
Copy link

cremor commented Oct 15, 2024

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()
@cremor cremor added the bug label Oct 15, 2024
@tmenier
Copy link
Owner

tmenier commented Oct 15, 2024

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
Copy link
Author

cremor commented Oct 15, 2024

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
Copy link
Owner

tmenier commented Oct 15, 2024

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
Copy link
Author

cremor commented Oct 15, 2024

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
Copy link
Owner

tmenier commented Oct 15, 2024

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.

@tmenier tmenier added enhancement and removed bug labels Oct 15, 2024
@tmenier tmenier changed the title Flurl.Http requires System.Text.Json even if Flurl.Http.Newtonsoft is used Avoid loading System.Text.Json when it isn't used Oct 15, 2024
@cremor
Copy link
Author

cremor commented Oct 15, 2024

No, my main concern is packaging.

  • Because I don't use any .NET packaging (which would include all dependencies automatically) I'd have to manually specify the whole STJ dependency tree in my setup project.
  • For the same reason I'd have to keep the file list up to date for any dependency change in the tree.
  • I'd have to release additional updates for my application any time STJ, or any of its dependencies, gets a security update.

But the reduced memory footprint is a nice bonus.

@tmenier
Copy link
Owner

tmenier commented Oct 15, 2024

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
Copy link
Author

cremor commented Oct 15, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

No branches or pull requests

2 participants