What is good practice when it comes to HttpClient in C#

Question

I've been learning C# over the past month and started to learn about HTTP requests using the HttpClient class. Right now I have a basic controller in my MVC project

private readonly HttpClient _client = new HttpClient();
private HttpResponseMessage response = new HttpResponseMessage();

public IActionResult Index() {
  return View();
}

[HttpGet("/data")]
async public Task < string > Data(int ? postId) {

  if (postId != null) {
    response = await _client.GetAsync("https://jsonplaceholder.typicode.com/comments?postId=" + postId);
  } else {
    response = await _client.GetAsync("https://jsonplaceholder.typicode.com/comments");
  }

  if (response.IsSuccessStatusCode) {
    return response.Content.ReadAsStringAsync().Result;
  }
  return "There was an error!";
}

This works perfectly fine, except I wanted to know if it's the 'right' way of doing things. I initiated the HttpClient and HttpResponseMessage when the controller class in initialized when the web app starts up, but should I do this inside the GET route? I read somewhere on Microsoft's website it's good to initialize only once per application.

https://learn.microsoft.com/en-us/aspnet/core/fundamentals/http-requests?view=aspnetcore-7.0 Does this help? (You are correct in being suspicious of newing up that HttpClient). The way you've done it is harder to configure, and iirc it's also susceptible to errors with DNS changes. — Ben Wong, Feb 03 '23 at 00:53

pigeonhands · Answer 1 · 2023-02-03T01:10:52.317

The methods you are using on HttpClient are thread safe, so you are correct in that you should only initialize it once. Even better in a singleton that handles the api requests.

However, HttpResponseMessage should be stored on a per-use basis to avoid nasty race conditions or unintentional side-effects.

In this case race-conditions are not an issue because asp creates a new instance of the controller for every request by default, but keeping values in the smallest scope is good practice to avoid unintentional side-effects by accidentally overwriting the value.

For example:

private async Task<string> CallOtherExternalApi() {
    response = await _client.GetAsync("https://example.com");
    return await response.Content.ReadAsStringAsync();
}

[HttpGet("/data")]
public async Task <string> Data(int? postId) {

  response = await _client.GetAsync("https://jsonplaceholder.typicode.com/comments");

  var otherData = await CallOtherExternalApi(); // response got overwritten

   // Now this code is checking the status of `CallOtherExternalApi`
  if (response.IsSuccessStatusCode) {
    return response.Content.ReadAsStringAsync().Result;
  }
  return "There was an error!";
}

What is good practice when it comes to HttpClient in C#

1 Answers1