dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.12k stars 9.91k forks source link

Better diagnostics/information for circular DI dependencies in Blazor WebAssembly #43866

Open Webreaper opened 1 year ago

Webreaper commented 1 year ago

Is there an existing issue for this?

Is your feature request related to a problem? Please describe the problem.

I have a bunch of scoped services in my Blazor WebAssembly app, and inadvertently added a cyclic dependency between two services:

When I launched my WASM app, after the loading progress bar, the app just hung. There was no logging, no information in the Chrome dev tools console, no exception, nothing - it just hung with no clue as to what was wrong. Luckily, after spending a bit of time looking at the commit I realised my mistake, but it would be useful if there were some clues for a developer. I think the lack of logging in DevTools is due to an infinite loop in the WebAssembly, meaning that the whole browser tab locks up.

Describe the solution you'd like

The page should fail to load with a more meaningful error/exception indicating a cyclic DI dependency, so it becomes immediately obvious where the eproblem lies.

Additional context

I don't think it makes a difference, but this issue was found running the WebAssembly app in Chrome, on MacOS, and the app was built with .Net 7 Preview 7.

javiercn commented 1 year ago

@Webreaper thanks for contacting us.

This is a feature of the dependency injection container, not a Blazor feature. You can try and use some of the options when building the container to validate your configuration. See https://docs.microsoft.com/en-us/dotnet/api/microsoft.extensions.dependencyinjection.serviceprovideroptions?view=dotnet-plat-ext-6.0#properties for details.

Specifically about calling BuildServiceProvider and passing in ValidateOnBuild = true

If that does not work for you, we recommend you file an issue in the dotnet/runtime repo which is where the DI container implementation lives.

Webreaper commented 1 year ago

Okay, thanks - I'll try the ValidateOnBuild setting.

I guess the reason I was flagging this up is because in a non-Webassembly app, if I create a cyclic dependency, I get an exception in the logs and/or the application crashes with the exception - so I know exactly what the cause is. Whereas in WebAssembly, because the hang blocks the browser from doing anything, there's literally no clue that there's even an issue related to DI at all. I wouldn't even know to add the ValidateOnBuild because I had no indication it was anything to do with DI dependencies.

So perhaps my suggestion here should be changed to "Default ServiceProviderOptions.ValidateOnBuild to true in a WebAssembly app, so cyclic failures are flagged to the developer before the process hangs the browser". Even if it was defaulted to true when app.Environment.IsDevelopment(), it might at least save some developers from wasting hours trying to figure out why their app hangs at startup.

Webreaper commented 1 year ago

One other question: when I try and add this in my WebAssembly app:

        builder.Host.UseDefaultServiceProvider( options =>
        {
            options.ValidateOnBuild = true;
        } );

there is no builder.Host property. I can't find any examples or documentation on how/where I add in the ValidateOnBuild flag for a webassembly app. Do you have a code snippet or a link to a sample?

Edit: So I figured out that I have to do something like this:

        builder.Services.BuildServiceProvider(  new ServiceProviderOptions {  ValidateOnBuild = true } );

but that builds me a second IServiceProvider, which is not what I want. How/where can I modify the service provider created within

        var builder = WebAssemblyHostBuilder.CreateDefault(args);

to add the ValidateOnBuild flag? :)

Edit #2: Seems like the only way to do this is to create a new service provider factory (example here) - which is a bit painful just to enable ServiceCollection validation. I wonder if the API could be improved to make this simpler to enable? Maybe one for @davidfowl ?

javiercn commented 1 year ago

@Webreaper Thanks for the additional details.

It might be a bit more involved in Blazor webassembly. You might need to call ConfigureContainer on the host passing in an instance of new DefaultServiceProviderFactory(options)

As to where the DI errors are surfaces, that is highly dependent on the context of the application type and the call site, it might be a bit strange that the app just hangs.

davidfowl commented 1 year ago

@javiercn maybe the web assembly host should make this easier.

javiercn commented 1 year ago

@davidfowl is the experience not consistent with how it works in ASP.NET Core? I am ok making things better, but I want to make sure we are consistent across the board.

davidfowl commented 1 year ago

ValidateOnBuild is true in the development environment and there are helpers that let you configure the default service provider factory options within creating a new DefaultServiceProviderFactory manually.

Webreaper commented 1 year ago

Interesting. I'm running in development and the browser hung, and I got no validation errors on startup.

Do you have any examples of how to configure without needing to use a factory?

javiercn commented 1 year ago

@Webreaper I think @davidfowl is referring to the experience in ASP.NET Core.

I am going to change this for an enhancement.

Triage: The work here is to include the same helpers we offer for this in ASP.NET Core.

@davidfowl could you point to the concrete extension methods?

ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

davidfowl commented 1 year ago

@davidfowl could you point to the concrete extension methods?

https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.hosting.webhostbuilderextensions.usedefaultserviceprovider?view=aspnetcore-6.0

javiercn commented 1 year ago

@davidfowl thanks.

The work here would be to add a new class WebassemblyHostBuilderExtensions and a method for the WebassemblyHostBuilder.UseDefaultServiceProvider that mimics https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.hosting.webhostbuilderextensions.usedefaultserviceprovider?view=aspnetcore-6.0 here

ghost commented 1 year ago

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

MattyLeslie commented 5 months ago

I'm going to look into implementing this enhancement.