29

I'm getting this exception using http client in my api.

An unhandled exception has occurred while executing the request. System.InvalidOperationException: This instance has already started one or more requests. Properties can only be modified before sending the first request.

and I injected my service as

services.AddSingleton<HttpClient>()

I thought singleton was my best bet. what could be my problem?

edit: my usage

class ApiClient
{
   private readonly HttpClient _client;
   public ApiClient(HttpClient client)
   {
      _client = client;
   }

   public async Task<HttpResponseMessage> GetAsync(string uri)
   {
     _client.BaseAddress = new Uri("http://localhost:5001/");
     _client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json");
     var response = await _client.GetAsync(uri);

     return response;
   }
 }
rethabile
  • 3,029
  • 6
  • 34
  • 68
  • 2
    Maybe post the whole class? We have no idea what is going on at the moment. – Luud van Keulen Feb 14 '17 at 20:34
  • Use `AddScoped` instead to get a different instance by request. – Kalten Feb 14 '17 at 20:39
  • 1
    Message is clear, once you set properties like BaseAddress and send out requests you cannot alter those properties afterwards. So singleton is ok, but only if you set the properties once as well. – Peter Bons Feb 14 '17 at 21:02
  • I'll be calling various services/endpoints, and would need to set `BaseAddress` accordingly. Does that rule out `Singleton`? – rethabile Feb 14 '17 at 21:08
  • @Kalten `AddScoped` is not recommended if the properties are being modified and multiple requests within the same scope will cause the exception thrown above. – Nico Feb 14 '17 at 21:09
  • @no0b i would recommend the `AddTransient` or create a custom factory (as per answer) – Nico Feb 14 '17 at 21:10

3 Answers3

59

This is the design of the class HttpClient .Net Core Source.

The interesting method here is the CheckDisposedOrStarted().

private void CheckDisposedOrStarted()
{
     CheckDisposed();
     if (_operationStarted)
     {
         throw new InvalidOperationException(SR.net_http_operation_started);
     }
}

Now this is called when setting the properties

  1. BaseAddress
  2. Timeout
  3. MaxResponseContentBufferSize

So if you are planning to reuse the HttpClient instance you should setup a single instance that presets those 3 properties and all uses must NOT modify these properties.

Alternativly you can create a factory or use simple AddTransient(...). Note that AddScoped is not best suited here as you will recieve the same instance per request scope.

Edit Basic Factory

Now a factory is nothing more than a service that is responsible for providing an instance to another service. Here is a basic factory to build your HttpClient now realize this is only the most basic you can extend this factory to do as you wish and presetup every instance of the HttpClient

public interface IHttpClientFactory
{
    HttpClient CreateClient();
}

public class HttpClientFactory : IHttpClientFactory
{
    static string baseAddress = "http://example.com";

    public HttpClient CreateClient()
    {
        var client = new HttpClient();
        SetupClientDefaults(client);
        return client;
    }

    protected virtual void SetupClientDefaults(HttpClient client)
    {
        client.Timeout = TimeSpan.FromSeconds(30); //set your own timeout.
        client.BaseAddress = new Uri(baseAddress);
    }
}

Now why did I use and interface? This is done as using dependency injection and IoC we can easily "swap" parts of the application out very easily. Now instead of trying to access the HttpClientFactory we access the IHttpClientFactory.

services.AddScoped<IHttpClientFactory, HttpClientFactory>();

Now in your class, service or controller you would request the factory interface and generate an instance.

public HomeController(IHttpClientFactory httpClientFactory)
{
    _httpClientFactory = httpClientFactory;
}

readonly IHttpClientFactory _httpClientFactory;

public IActionResult Index()
{
    var client = _httpClientFactory.CreateClient();
    //....do your code
    return View();
}

The key here is.

  1. The factory is responsible for generating the client instance and will manage the defaults.
  2. We are requesting the interface not the implementation. This helps us keep our components disconnected and allow for a more modular design.
  3. The service is registered as a Scoped instance. Singletons have their uses but in this case you are more likely to want a scoped instance.

Scoped lifetime services are created once per request.

Nico
  • 12,493
  • 5
  • 42
  • 62
  • 9
    Creating a new client per request will lead to port exhaustion. – Tratcher Jan 18 '18 at 15:53
  • @Tratcher yes it can but not to port exhaustion but to socket exhaustion. Anyway, this was purely for an example. Others should read related questions and posts such as https://aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/ – Nico Jan 18 '18 at 21:20
  • 9
    People have a habit of copying bad examples, please take it down. – Tratcher Jan 19 '18 at 02:07
  • 1
    ref: https://aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/ and https://ankitvijay.net/2016/09/25/dispose-httpclient-or-have-a-static-instance/ – ablaze Apr 08 '19 at 14:53
  • 1
    This answer is well-meaning, but perhaps ill-suited: the example code exemplifies how to not use the HttpClient. The scope has to be Singleton in this particular case, otherwise, as has been pointed out, we'll get socket exhaustion. – Mir Oct 13 '19 at 12:31
15

Singletons are the correct approach. Using scoped or transient will prevent connection pooling and lead to perf degradations and port exhaustion.

If you have consistent defaults then those can be initialized once when the service is registered:

        var client = new HttpClient();
        client.BaseAddress = new Uri("http://example.com/");
        client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
        services.AddSingleton<HttpClient>(client);

...

        var incoming = new Uri(uri, UriKind.Relative); // Don't let the user specify absolute.
        var response = await _client.GetAsync(incoming);

If you don't have consistent defaults then BaseAddress and DefaultRequestHeaders should not be used. Create a new HttpRequestMessage instead:

        var incoming = new Uri(uri, UriKind.Relative); // Don't let the user specify absolute urls.
        var outgoing = new Uri(new Uri("http://example.com/"), incoming);
        var request = new HttpRequestMessage(HttpMethod.Get, outgoing);
        request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
        var response = await _client.SendAsync(request);
Tratcher
  • 5,929
  • 34
  • 44
  • My application interacts with several (let's say a dozen) APIs on a few (let's say five) different domains. Is it appropriate to spin up a separate singleton HttpClient object in my DI container for each API? For each domain? Or should I make a separate HttpRequestMessage for each API? – Dan Narsavage Feb 03 '20 at 22:37
  • 1
    Whichever is most convenient, it won't affect the behavior. Internally HttpClient groups connections by domain. As long as you reuse the same client for a given domain, it doesn't matter if you use one client for multiple domains (until you need different settings). – Tratcher Feb 04 '20 at 02:16
  • I'm just going to leave this link to the .Net Core v3.1 docs here for reference. `https://learn.microsoft.com/en-us/dotnet/api/system.net.http.httpclient?view=netcore-3.1#examples` The first line of the example code states; `// HttpClient is intended to be instantiated once per application, rather than per-use. See Remarks. (\r\n) static readonly HttpClient client = new HttpClient(); ...` – Scott Fraley Jul 16 '20 at 18:08
0

It's preferable to add the url of your request and the headers at the message, rather than on the client. better not to use the BaseAddress or DefaultRequestHeaders unless you have have to.

HttpRequestMessage yourmsg = new HttpRequestMessage {
    Method = HttpMethod.Put,
    RequestUri = new Uri(url),
    Headers = httpRequestHeaders;
};


httpClient.SendAsync(yourmsg);

It Works well for reusing a single HttpClient for many requests

Abraham
  • 12,140
  • 4
  • 56
  • 92