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

DefaultHttpClientFactory working incorrectly with Scoped dependencies #42344

Closed
mrpmorris opened this issue Jul 13, 2020 · 5 comments
Closed
Labels
arch-wasm WebAssembly architecture area-Extensions-HttpClientFactory enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@mrpmorris
Copy link

mrpmorris commented Jul 13, 2020

Describe the bug

When using Microsoft.Extensions.Http AddHttpClient and AddHttpMessageHandler, the instance of T belongs to a different ServiceProvider than the ServiceProvider used to generate a Blazor page - even though T is registered as scoped.

This seems to be a problem with DefaultHttpClientFactory being registered as Singleton instead of Scoped. In Blazor every user connection equates to a Scoped container, so when our Http handling delegates have dependencies we really need them to be created in the correct Scoped container.

The Blazor team suggested this should be reported here. Perhaps we could have the option of registering it scoped or something?

dotnet/aspnetcore#23847

To Reproduce

Create a new Blazor WASM project

Add NuGet reference Microsoft.Extensions.Http

Create a class as follows

	public class MyDelegatingHandler : DelegatingHandler
	{
		private readonly IServiceProvider ServiceProvider;

		public MyDelegatingHandler(IServiceProvider serviceProvider)
		{
			ServiceProvider = serviceProvider;
		}

		protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
		{
			Console.WriteLine("Http service provider hash = " + ServiceProvider.GetHashCode());
			return base.SendAsync(request, cancellationToken);
		}
	}

In Program.cs remove the registration of HttpClient and replace it with the following

builder.Services.AddScoped<MyDelegatingHandler>();
builder.Services
    .AddHttpClient("local", c => c.BaseAddress = new Uri(builder.HostEnvironment.BaseAddress))
    .AddHttpMessageHandler<MyDelegatingHandler>();

Edit FetchData.razor
Remove the injection of HttpClient and relace it with

@inject IHttpClientFactory HttpClientFactory
@inject IServiceProvider ServiceProvider

Replace OnInitializedAsync with the following code

protected override async Task OnInitializedAsync()
{
	var httpClient = HttpClientFactory.CreateClient("local");
	Console.WriteLine("Page service provider hash = " + ServiceProvider.GetHashCode());
	forecasts = await httpClient.GetFromJsonAsync<WeatherForecast[]>("sample-data/weather.json");
}

Now run the app, open the console window in the browser, and navigate to the Fetch Data page.

Expected

The hash code for the page and the http delegate should be the same.

Actual

They are different, therefore they are both running within separate IServiceProvider containers.

@BrennanConroy BrennanConroy transferred this issue from dotnet/extensions Sep 16, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-Extensions-HttpClientFactory untriaged New issue has not been triaged by the area owner labels Sep 16, 2020
@ghost
Copy link

ghost commented Sep 16, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@ericstj ericstj added arch-wasm WebAssembly architecture enhancement Product code improvement that does NOT require public API changes/additions and removed untriaged New issue has not been triaged by the area owner labels Sep 18, 2020
@ericstj ericstj added this to the 6.0.0 milestone Sep 18, 2020
@ericstj
Copy link
Member

ericstj commented Sep 18, 2020

cc @dotnet/ncl

@cssack
Copy link

cssack commented Oct 1, 2020

Is there anyone who found a way to get the service provider used by the page within a DelegatingHandler?

@CarnaViire
Copy link
Member

Triage: the root problem here is that message handlers are currently created in a manual scope (unrelated to existing scopes). Closing this one as a duplicate of #36574

@CarnaViire
Copy link
Member

Duplicate of #36574

@CarnaViire CarnaViire marked this as a duplicate of #36574 Feb 1, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Mar 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
arch-wasm WebAssembly architecture area-Extensions-HttpClientFactory enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

5 participants