I have an old version of ASP.NET MVC app that doesn't have a Startup.cs. I wanted to implement a clean way to have an HttpClient that I would use for my API calls to third parties.
Here's what I've done so far based on some ideas/recommendations I've received for this question. The problem is that when I make the API call, it goes nowhere. I put it in a try catch but I'm not even getting an exception. The API provider tells me that they're not seeing the search parameter.
First, I created this HttpClientAccessor for lazy loading.
public static class HttpClientAccessor { public static Func<HttpClient> ValueFactory = () => { var client = new HttpClient(); client.BaseAddress = new Uri("https://apiUrl.com"); client.DefaultRequestHeaders.Accept.Clear(); client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); client.DefaultRequestHeaders.TryAddWithoutValidation("APIAccessToken", "token1"); client.DefaultRequestHeaders.TryAddWithoutValidation("UserToken", "token2"); return client; }; private static Lazy<HttpClient> client = new Lazy<HttpClient>(ValueFactory); public static HttpClient HttpClient { get { return client.Value; } } } I then created an API client of my own so that I can have the API call functions in one place which looks like this:
public class MyApiClient { public async Task GetSomeData() { var client = HttpClientAccessor.HttpClient; try { var result = await client.GetStringAsync("somedata/search?text=test"); var output = JObject.Parse(result); } catch(Exception e) { var error = e.Message; } } } Then in my ASP.NET Controller action, I do this:
public class MyController : Controller { private static readonly MyApiClient _apiClient = new MyApiClient (); public ActionResult ApiTest() { var data = _apiClient.GetSomeData().Wait(); } } Any idea where my mistake is?
UPDATE: This simple approach works fine:
public class MyController : Controller { private static readonly HttpClient _client = new HttpClient(); public ActionResult ApiTest() { _client.BaseAddress = new Uri("https://apiUrl.com"); _client.DefaultRequestHeaders.Accept.Clear(); _client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); _client.DefaultRequestHeaders.TryAddWithoutValidation("APIAccessToken", "token1"); _client.DefaultRequestHeaders.TryAddWithoutValidation("UserToken", "token2"); var response = _client.GetStringAsync("somedata/search?text=test").Result; } } 4 Answers
Answers 1
As mentioned, dependency injection is not being utilized so technically there is no need for a composition root where these things would have been initialized.
If there is no need to actually initialize the client on start up you could consider using a Lazy singleton approach.
An example
public static class HttpClientAccessor { public static Func<HttpClient> ValueFactory = () => { var client = new HttpClient(); client.BaseAddress = new Uri("https://apiUrl.com"); client.DefaultRequestHeaders.Accept.Clear(); client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); client.DefaultRequestHeaders.TryAddWithoutValidation("APIAccessToken", "token1"); client.DefaultRequestHeaders.TryAddWithoutValidation("UserToken", "token2"); return client; }; private static Lazy<HttpClient> client = new Lazy<HttpClient>(ValueFactory); public static HttpClient HttpClient { get { return client.Value; } } } The factory delegate of the Lazy<HttpClient> can be made more complex if additional settings are needed on the client.
And where ever the client is needed you call the service
var client = HttpClientAccessor.HttpClient; var response = await client.GetStringAsync("{url}"); the client will be initialized on first use and you will get the same instance on subsequent calls for the instance.
As used in your controller, you are mixing async calls with blocking calls line .Wait() or .Result. This can lead to deadlocks and should be avoided.
public class MyController : Controller { private static readonly MyApiClient _apiClient = new MyApiClient (); public async Task<ActionResult> ApiTest() { var data = await _apiClient.GetSomeData(); //... } } Code should be async all the way through.
Reference Async/Await - Best Practices in Asynchronous Programming
Answers 2
The Application_Start() method is the right place. But I would have to ask: why you have to create the HttpClient instance when the "application starts"? In general, HttpClient is some "resource" and you can just create it when you want to use it. And also it's no need to set it as "Singleton". Just wrap it in the using block. (Maybe you want to make the API wrapper as Singleton?)
public class APICaller { //make the APICaller singleton in some way here //... // the api calling method: public string CallAPI(string someParameter) { var response = ""; using (var client = new HttpClient()) { //calling the API } return response; } } Answers 3
The main issue is incorrect asynchronous code.
You are using Task.Wait() which alongside asynchronous MyApiClient.GetSomeData() causes a deadlock on ASP.NET request context. That is a very common issue, see An async/await example that causes a deadlock on StackOverflow. Code with Task.Result property call is working because HttpClient.GetStringAsync() probably takes preventative measures against deadlocks. See Task.ConfigureAwait() page on MSDN and Best practice to call ConfigureAwait for all server-side code discussion on StackOverflow.
There are multiple options to write a singleton using C#. See Implementing the Singleton Pattern in C# article by Jon Skeet for a detailed overview.
Answers 4
As you mentioned, you can just use a static class member on the controller. HttpClient only needs to be setup once; so do this in the static constructor of the controller. Also, make sure that you use async/await for async methods, especially with long running http requests. IOC and an abstraction layer would make sense depending on your needs.
using System; using System.Net.Http; using System.Threading.Tasks; namespace TestApi { public class MyController : Controller { private const string ApiUrlString = "https://apiUrl.com"; private static readonly Uri ApiUri = new Uri(ApiUrlString); private static readonly HttpClient RestClient; static MyController() { this.RestClient = new HttpClient{ BaseAddress = ApiUri } this.RestClient.DefaultRequestHeaders.Accept.Clear(); this.RestClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); RestClient.DefaultRequestHeaders.TryAddWithoutValidation("APIAccessToken", "token1"); RestClient.DefaultRequestHeaders.TryAddWithoutValidation("UserToken", "token2"); } public async Task<IActionResult> ApiTest() { return this.Ok(await this.RestClient.GetStringAsync("somedata/search?text=test")); } } } 
