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

Improve Csla.Channels.Http.HttpProxy #4431

Open
ctescu opened this issue Jan 8, 2025 · 5 comments
Open

Improve Csla.Channels.Http.HttpProxy #4431

ctescu opened this issue Jan 8, 2025 · 5 comments

Comments

@ctescu
Copy link

ctescu commented Jan 8, 2025

HttpProxy uses WebClient for all sync call.
WebClient is marked like obsolete https://learn.microsoft.com/en-us/dotnet/api/system.net.webclient?view=net-8.0 and suggested to be replaced with sync calls from HttpClient.

@rockfordlhotka
Copy link
Member

Thank you @ctescu, this will be a good enhancement for more modern versions of .NET.

We'll need to use compiler directives to continue using WebClient for netfx targets (4.6.2, 4.7.2, 4.8) I assume?

@StefanOssendorf
Copy link
Contributor

I'm not so sure about that. Yeah there is a Send() method on HttpClient but no extension methods like PostAsync, GetAsync and so on as sync versions. And I'm not sure if we should implement such methods for our usage 🤔.

If we plan to implement that we should consider making this an opt-in feature. Currently some stuff for WebClient is configured via ServicePointManager which is not used in any way by HttpClient. So that might be more impactfull than just changing our implementation.

@ctescu
Copy link
Author

ctescu commented Jan 9, 2025

Dear @StefanOssendorf now DefaultWebClient uses req.Timeout = timeout.Milliseconds; that will trigger the exception that timeout must be greater than 0 or infinite,.
I don't think the current implementation works as expected, for me to work with sync call I needed to create a new WebClient and use TotalMiliseconds.
For async call I had the problem from https://www.aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/ and changed the implementation to https://josef.codes/you-are-probably-still-using-httpclient-wrong-and-it-is-destabilizing-your-software/ with [IHttpClientFactory] .

@StefanOssendorf
Copy link
Contributor

Thanks for pointing the bug out in our current sync calls. I've created an issue to fix that.

But I don't get the part with the async call problem. We have since v8 IIRC, this option for the HttpProxy:

/// <summary>
/// Gets or sets the factory to obtain an <see cref="HttpClient"/> to make server calls.
/// </summary>
public Func<IServiceProvider, HttpClient> HttpClientFactory { get; set; } = DefaultHttpFactory;

and we are using it to setup the HttpProxy
var client = proxyOptions.HttpClientFactory(sp);
return new HttpProxy(applicationContext, client, proxyOptions, dataPortalOptions);

So if you want to use the IHttpClientFactory (which I do at work) you only have to configure it's usage in the registration and you are done.

@ctescu
Copy link
Author

ctescu commented Jan 10, 2025

You are right about async call, was a misusage in my code.

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

No branches or pull requests

3 participants