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

Bug: The way httpclient is being used in the MandrillApi could lead to several issues #218

Open
codingmoh opened this issue Mar 22, 2020 · 0 comments

Comments

@codingmoh
Copy link

codingmoh commented Mar 22, 2020

Hello there,

while browsing through the source code I've noticed, that the httpclient is being instantiated in the constructor of the MandrillApi, this could be causing several issues, like socket exhaustion and outdated dns configuration.

From MSDN Documentation:

While this class implements IDisposable, declaring and instantiating it within a using statement is not preferred because when the HttpClient object gets disposed of, the underlying socket is not immediately released, which can lead to a socket exhaustion problem. For more information about this issue, see the blog post You're using HttpClient wrong and it's destabilizing your software.

See: https://docs.microsoft.com/en-us/dotnet/architecture/microservices/implement-resilient-applications/use-httpclientfactory-to-implement-resilient-http-requests#issues-with-the-original-httpclient-class-available-in-net-core
And here: https://aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/

Another potential issue I've discovered is that the HttpClient is being used without a retry policy. I/O Operations and request (esp. HttpRequests) could always fail due to various reasons. This could be easily solved by using the polly library:
https://docs.microsoft.com/en-us/dotnet/architecture/microservices/implement-resilient-applications/implement-http-call-retries-exponential-backoff-polly

Suggestion for fixing this:

  • Either inject httpclientfactory or httpclient
  • Use Polly library to allow automatic retries
@codingmoh codingmoh changed the title Bug: HttpClient Usage and usage without retry policy Bug: The way httpclient is being used could cause several issues Mar 22, 2020
@codingmoh codingmoh changed the title Bug: The way httpclient is being used could cause several issues Bug: The way httpclient is being used, could cause several issues Mar 22, 2020
@codingmoh codingmoh changed the title Bug: The way httpclient is being used, could cause several issues Bug: The way httpclient is being used in the MandrillApi could lead to several issues Mar 22, 2020
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

1 participant