Allow HostFactoryResolver to listen on arbitrary events

[Youssef1313]

I needed this to prototype the change in dotnet/aspnetcore#66527.

Whether or not we will need to use that in aspnet is to be decided when dotnet/aspnetcore#66574 is reviewed.

The change here is in internal class which is consumed via an internal source-only package (that package isn't shipped to nuget.org). The PR doesn't introduce any behavior changes to runtime. It will only allow aspnetcore to hook additional delegates, which will allow us to fire HostApplicationBuilderCreated event when the user calls WebApplication.CreateBuilder, and gives us the opportunity in the testing library to add early configuration sources that users expect to be available before Build() is called.

[dotnet-policy-service]

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

[Copilot]

Pull request overview

This PR extends HostFactoryResolver.ResolveHostFactory to allow callers to attach callbacks for additional DiagnosticSource event names emitted by Microsoft.Extensions.Hosting, enabling tooling scenarios that need to observe more than just the existing HostBuilding / HostBuilt flow.

Changes:

• Adds an optional arbitraryActions map to ResolveHostFactory and threads it into HostingListener.
• Invokes a registered callback when an observed hosting DiagnosticSource event key matches an entry in arbitraryActions.
• Adds a new using Microsoft.Extensions.Configuration; directive (currently unused).

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

[svick]

Mostly repeating what copilot said, but:

1. I think there should be tests for this.
2. You should make sure that exceptions from arbitraryActions don't escape to the thread pool. I.e. if you're using this in a test and an arbitraryAction throws, it should fail the test, not crash the test process. Maybe it alwady works that way, I don't know.

[svick]

@Youssef1313 If you're not planning on getting this PR merged soon, can you turn it into a draft? We don't want to have PRs that are open for a long time. Thanks.

[Youssef1313]

@svick With regards to exceptions, how is the new arbitraryActions different from the existing configureHostBuilder which is just called as-is?

[github-actions]

Caution

Security scanning requires review for Code Review

Details

The threat detection results could not be parsed. The workflow output should be reviewed before merging.

Review the workflow run logs for details.

🤖 Copilot Code Review — PR #127743

Holistic Assessment

Motivation: The PR adds a hook mechanism to allow consumers (ASP.NET Core) to listen on arbitrary diagnostic source events during host resolution. The use case (firing HostApplicationBuilderCreated for testing libraries) is reasonable, though the aspnetcore design is still under review.

Approach: Adding an IDictionary<string, Action<object?>> parameter to an internal API is a straightforward extension. The approach is minimal and consistent with how configureHostBuilder already works.

Summary: ⚠️ Needs Human Review. The code change is small and mechanically correct, but there are two concerns that a human reviewer should weigh: (1) missing tests, and (2) a subtle behavioral change in the if/else if restructuring.

---

Detailed Findings

⚠️ Missing Tests

The reviewer @svick already flagged this. This PR adds new functionality with no test coverage. Even though this is an internal source-only package, the HostFactoryResolver has existing tests (in Microsoft.Extensions.Hosting.Tests or similar). A test verifying that arbitrary actions are invoked for matching diagnostic keys would be appropriate.

Classification: Advisory — the aspnetcore consumer will implicitly test this, but in-repo tests are standard practice.

⚠️ Behavioral Change: if → else if for "HostBuilding"/"HostBuilt"

The original code had:

if (value.Key == "HostBuilding") { ... }

if (value.Key == "HostBuilt") { ... }

This was changed to if / else if / else if. Since value.Key can only be one string at a time, this is semantically equivalent for these two cases — only one branch could ever match per call. However, it's worth noting explicitly that this is safe because OnNext is called once per event, not with a key that could match both.

The else if for _arbitraryActions correctly ensures that if a consumer passes "HostBuilding" or "HostBuilt" as keys in the dictionary, they will not fire (the built-in handling takes precedence). This is reasonable but should be documented — a consumer could be surprised that these keys are silently ignored.

Classification: Advisory — the behavior is correct but the precedence rule should be called out in a doc comment or the method's XML docs.

✅ Exception Handling

The author's point in the comments is valid: arbitraryAction.Invoke(value.Value) has the same exception propagation characteristics as _configure?.Invoke(value.Value!). Both are called on the entry point thread inside a try/catch that captures exceptions into _hostTcs. Exceptions from arbitrary actions will propagate to the TrySetException path and surface to the caller, not crash the process.

💡 Naming: arbitraryActions

The name "arbitrary" is vague. Consider eventActions, diagnosticEventHandlers, or additionalEventListeners to better convey that these are keyed by diagnostic event names.

---

Generated by Copilot code review

Generated by Code Review for issue #127743 · ● 1.3M · ◷

[Copilot]

Mostly repeating what copilot said, but:

1. I think there should be tests for this.
   ...

Addressed in 08df3ae: added tests for arbitraryActions callback invocation and explicit exception propagation behavior so thrown callback exceptions fail the test path instead of escaping unchecked.

[svick]

With regards to exceptions, how is the new arbitraryActions different from the existing configureHostBuilder which is just called as-is?

It seems that with how it's used, an exception in arbitraryActions (or configureHostBuilder) bubbles up through the invoked Main and ends up failing the test. So I think it already behaves the way it should and you don't need to change anything.

[svick]

Note that I have added your new project file to the solution. I hope you don't mind.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

[Youssef1313]

API proposal on aspnetcore is approved. This should be ready for review again now.