search

braintree / braintree_dotnet

Braintree .NET library
https://developer.paypal.com/braintree/docs/start/overview
MIT License
136 stars 73 forks source link

Improve async for .NET 4.5 #55

Closed onlyann closed 7 years ago

onlyann commented 7 years ago

This pull request improves the implementation of the asynchronous operations for .NET 4.5 in the following ways:

sdcoffey commented 7 years ago

Hey @onlyann,

Thanks for the PR, and sorry for taking so long to follow up on it. Were you able to run the tests after making these changes? This looks on the surface like a breaking change, which means that we would have to wait until the next major version to release this.

onlyann commented 7 years ago

Hi @sdcoffey

Tests run fine on my end. Can you elaborate as to why you believe this is a breaking change? None of the contracts have changed and exception handling should be transparent.

sdcoffey commented 7 years ago

At first glance, changed method signatures seem like breaking changes. I'm no C Sharp expert though, so I could be wrong. Can you explain the motivation behind this change and what the tangible benefits are?

onlyann commented 7 years ago

the async keyword does not modify the signature of the methods.

This issue was already discussed in #32. It was well described in one of the replies:

Concurrency is not the same as async. An ASP.NET application is already naturally concurrent. Async/await improves scalability and throughput--threads do not block while waiting for external processes, e.g. the response from your server, and are free to handle other requests. I'd like to talk to you more about this. Not supporting async means that you are effectively artificially limiting the performance of your clients' applications.

At the moment, only a few IO calls are performed asynchronously which this PR tries to fix. The tangible benefits are that web applications consuming Braintree through this SDK can achieve better throughput because ASP.NET threads don't get unnecessarily blocked.

Mainly, the HTTP Request and Response to Braintree servers are blocking the threads before this PR. For instance, request.GetResponse is blocking while request.GetResponseAsync isn't.

iamcarbon commented 7 years ago

Please add these ConfigureAwaits() -- we're experiencing deadlocks on ASP.NET using the Async methods.

Thanks!

bluk commented 7 years ago

Thanks for the contribution! We'll put this in our next release.