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

HttpClient/Request problemi ve async/await kullanılamaması #74

Open
staviloglu opened this issue Feb 8, 2018 · 5 comments
Open

HttpClient/Request problemi ve async/await kullanılamaması #74

staviloglu opened this issue Feb 8, 2018 · 5 comments

Comments

@staviloglu
Copy link

Mevcut versiyondaki RestHttpClient objesi her request için yeniden bir HttpClient yaratıyor.

public T Post<T>(String url, WebHeaderCollection headers, BaseRequest request)
        {
            HttpClient httpClient = new HttpClient();
            foreach (String key in headers.Keys)
            {
                httpClient.DefaultRequestHeaders.Add(key, headers.Get(key));
            }
            HttpResponseMessage httpResponseMessage = httpClient.PostAsync(url, JsonBuilder.ToJsonString(request)).Result;
            return JsonConvert.DeserializeObject<T>(httpResponseMessage.Content.ReadAsStringAsync().Result);
        }

HttpClient objesi bir kere oluşturup çok sefer kullanmak için tasarlanmış bir nesne olmasından dolayı kullanıcıların canını sıkacak durumlar oluşabilir diye düşünüyorum. Bu konuda aşağıdaki linkler değerlendirilebilir.
Improper Instantiation antipattern
HttpClient per call in a Web API client

Ayrıca HttpClient'ın async fonksyionları Task.Result kullanılarak çağıran threadi bloklayacak şekilde kullanılmış. Buna alternatif olarak async/await kullanılması ASP.NET kullanıcılarına faydalı olacaktır diye düşünüyorum.
Async Programming : Introduction to Async/Await on ASP.NET

Task.Result Property

Accessing the property's get accessor blocks the calling thread until the asynchronous operation is complete; it is equivalent to calling the Wait method.

Async and Await

I like to think of “await” as an “asynchronous wait”. That is to say, the async method pauses until the awaitable is complete (so it waits), but the actual thread is not blocked (so it’s asynchronous).

@staviloglu staviloglu mentioned this issue Feb 8, 2018
@akselarzuman
Copy link

Merhaba,

Kendi client library'mizde bunu düzeltmiş bulunmaktayız.

https://github.com/armutcom/iyzipay-dotnet-client

@staviloglu
Copy link
Author

@akselarzuman selamlar.
Problemi görünce kullananlara faydası olması açısından burada issue açıp çözüm önerisini de paylaşmıştım zaten PullRequest#75 Gerçi 8 Şubattan beri bekliyor ama... :)
Sizin de kendi client'ınızı güncellemeniz çok güzel ama şahsi görüşüm iyzico kullananların ilk baktıkları yer burası olması dolayısı ile buradaki kodun güncellenmesinin daha faydalı olacağı yönünde.
Paylaşım için teşekkürler 👍 , iyi çalışmalar.

@ibrahimozgon
Copy link

Biz bahsettiğiniz 2 numaralı sorunu canlıda yasıyoruz. @enerefe
System.AggregateException: One or more errors occurred. ---> System.Threading.Tasks.TaskCanceledException: A task was canceled. --- End of inner exception stack trace --- at System.Threading.Tasks.Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions) at System.Threading.Tasks.Task1.GetResultCore(Boolean waitCompletionNotification) at Iyzipay.RestHttpClient.Post[T](String url, Dictionary2 headers, BaseRequest request) at Iyzipay.Model.Payment.Create(CreatePaymentRequest request, Options options) at Arabam.Core.Services.Payments.Implementations.IyzicoPaymentV2Service.PayWithOtp(OtpPaymentRequest request) ---> (Inner Exception #0) System.Threading.Tasks.TaskCanceledException: A task was canceled.<---
Iyzipay 2.1.33.0 versiyonu kullanıyoruz.

@akselarzuman Iyzipay yakın zamanda son bir versiyon çıkmış ancak eski yapı tamamen değişmiş gözüküyor. Bundan sonrası için Armut iyzipay projesi farklı bir yoldan ilerleyecek diyebilir miyiz?

1 numara için ise; iyzipay her request icin yeni bir httpClient üretiyor ve dispose etmiyor, armut.com ise ctor'da HttpClient üretiyor ve dispose etmiyor.
Ancak microsoft ne singleton kullanımını ne de her istekte yeni httpClient olusturmayı desteklemiyor bildiğim kadarıyla:

Therefore, HttpClient is intended to be instantiated once and reused throughout the life of an application. Instantiating an HttpClient class for every request will exhaust the number of sockets available under heavy loads. That issue will result in SocketException errors. Possible approaches to solve that problem are based on the creation of the HttpClient object as singleton or static, as explained in this Microsoft article on HttpClient usage.

But there’s a second issue with HttpClient that you can have when you use it as singleton or static object. In this case, a singleton or static HttpClient doesn’t respect DNS changes, as explained in this issue at the .NET Core GitHub repo.

To address those mentioned issues and make the management of HttpClient instances easier, .NET Core 2.1 introduced a new HttpClientFactory that can also be used to implement resilient HTTP calls by integrating Polly with it.

Kaynak: https://dotnet.microsoft.com/download/e-book/microservices-architecture/pdf

@bugrakosen
Copy link

Projeyi forklayıp bütün bu konuları gözden geçirerek yeniden yazdık. Kendi kullanımımız için yaptık fakat buraya da bırakmak istedim. Sadece .Net 5 sürümünü destekliyor. Birazcık değiştirdik :)

İstek atılırken HttpClient harici nesneler cevap aldıktan sonra using statement'ı ile dispose ediliyor. Fakat HttpClient'ı tutan nesneyi manuel dispose etmeniz gerekli. Kullanım farklılıklarından dolayı çıkacak problemleri engellemek amaçlı bu şekilde yaptım.

Gözünüze çarpan eksiklerimizi söylerseniz memnun oluruz. Umarım faydalı olur.

https://github.com/Milvasoft/Milvasoft.Iyzipay

@LazZiya
Copy link

LazZiya commented Sep 6, 2023

Neredeyse 6 yıl geçmiş ama hala bir düzeltme yok ilgilenen de yok...

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

No branches or pull requests

5 participants