Move HostAbortedException to Abstractions#70205
Conversation
HostAbortedException was added directly to Microsoft.Extensions.Hosting, which depends on a lot of other libraries. This causes problems when using the HostFactoryResolver.Sources package because now consumers of HostFactoryResolver need to reference the whole Hosting package and all of its dependencies. Fix this by moving the new exception to the Hosting.Abstractions library, which has a minimal set of dependencies.
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
|
Tagging subscribers to this area: @dotnet/area-extensions-hosting Issue DetailsHostAbortedException was added directly to Microsoft.Extensions.Hosting, which depends on a lot of other libraries. This causes problems when using the HostFactoryResolver.Sources package because now consumers of HostFactoryResolver need to reference the whole Hosting package and all of its dependencies. Fix this by moving the new exception to the Hosting.Abstractions library, which has a minimal set of dependencies. I couldn't add a dependency from
|
stephentoub
left a comment
There was a problem hiding this comment.
Were there any tests that should be moved as well?
There are tests in |
It's fitting that my PR fails for the flaky test that I'm trying to fix in another PR |
|
Please see dotnet/aspnetcore#41999 (comment). Short story is this won't work in aspnetcore and violates the purpose of the .Sources package.
Can't do this w/o limiting an "inside man" library to 7.0.0-preview6 or later web sites. As I said in dotnet/aspnetcore#41999,
|
|
Imo we should introduce a test project for But everything else looks good to me. |
dougbu
left a comment
There was a problem hiding this comment.
Please find another way to make the Exception public
why? |
Because any 7.0.x dependencies makes the .Sources package useless in https://github.com/dotnet/aspnetcore/blob/main/src/Tools/GetDocumentInsider/src/GetDocument.Insider.csproj and massively limits the applicable scope of the Microsoft.Extensions.ApiDescription.Client package. Please see #70205 (comment) |
…ions.Hosting assemblies. Load the HostAbortedException using Reflection instead. If it can't be found, throw a private exception with the same name.
|
I have changed HostFactoryResolver to follow the suggestion in dotnet/aspnetcore#41999 (comment). This removes the direct dependency on Microsoft.Extensions.Hosting assemblies, which should address @dougbu's feedback above. |
| _hostTcs.TrySetException(new InvalidOperationException("The entry point exited without ever building an IHost.")); | ||
| } | ||
| catch (TargetInvocationException tie) when (tie.InnerException is HostAbortedException) | ||
| catch (TargetInvocationException tie) when (tie.InnerException?.GetType().Name == "HostAbortedException") |
There was a problem hiding this comment.
nit: Should this instead check the Exception's full name❔
There was a problem hiding this comment.
The private exception purposefully has a different full name (since it is nested). So we would need to check 2 different "full names".
Even full names wouldn't prevent some other code running in a Host "Build()" sequence naming their exception Microsoft.Extensions.Hosting.HostAbortedException. So it isn't a guaranteed solution either.
If, for some reason, someone else names their exception HostAbortedException, and it isn't the one we threw below, the caller will get null back from HostFactoryResolver.ResolveServiceProviderFactory, and it will handle it just like any other case where the IServiceProvider can't be found (usually displaying a message to the user saying something went wrong).
|
Merging to unblock aspnetcore taking a new runtime version. I can handle any follow up feedback in a separate PR. |
HostAbortedException was added directly to Microsoft.Extensions.Hosting, which depends on a lot of other libraries. This causes problems when using the HostFactoryResolver.Sources package because now consumers of HostFactoryResolver need to reference the whole Hosting package and all of its dependencies.
Fix this by moving the new exception to the Hosting.Abstractions library, which has a minimal set of dependencies.
I couldn't add a dependency from
HostFactoryResolver.SourcestoHosting.Abstractions. The packing infrastructure is getting in my way of doing that (cc @ViktorHofer). @dougbu - to consumeHostFactoryResolver.Sources, you will need to manually add a reference toMicrosoft.Extensions.Hosting.Abstractions.