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

Host: log exception if a service factory fails #112534

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mus65
Copy link

@mus65 mus65 commented Feb 13, 2025

Currently, the host doesn't log any exception if a IHostedService or IStartupValidator service factory fails. If you use RunAsync you also have no way to log it yourself since the host has already been disposed after it returns (see also #72225).

This catches any exceptions thrown by the GetService calls and logs and re-throws the exception, similiarly on how it is already done for exceptions in StartAsyncetc. .

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Feb 13, 2025
Copy link
Contributor

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// service factory or validation failed, abort startup.
exceptions.Add(ex);
LogAndRethrow();
return; // unreachable
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend either removing this return, or making it consistent with the rest of the code here.

There's also [DoesNotReturn] that can be applies to LogAndRethrow() but that would be just for readability in this case I believe.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return; is necessary here to avoid a null reference warning for _hostedServices below. [DoesNotReturn] is not possible because LogAndRethrow() does not actually throw if exceptions is empty.

@steveharter steveharter requested a review from halter73 February 14, 2025 20:15
@steveharter
Copy link
Member

cc @halter73 for any feedback. Thanks

@steveharter steveharter self-assigned this Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Extensions-Hosting community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants