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

Missing Logging of Exceptions in IHostedService without await #84559

Closed
Tripletri opened this issue Apr 10, 2023 · 4 comments
Closed

Missing Logging of Exceptions in IHostedService without await #84559

Tripletri opened this issue Apr 10, 2023 · 4 comments
Labels
area-Extensions-Hosting needs-author-action An issue or pull request that requires more info or actions from the author.
Milestone

Comments

@Tripletri
Copy link

Description

When an exception occures in IHostedService without calling await, it is not logged to the ILogger.
This make it difficult to identify the reason for the application shutdown when writing logs to an external service.

Reproduction Steps

IHost host = Host.CreateDefaultBuilder(args)
    .ConfigureServices(services =>
    {
        services.AddHostedService<Worker>();
    })
    .Build();

await host.RunAsync();

public class Worker : IHostedService
{
    public Task StartAsync(CancellationToken cancellationToken) =>
        throw new Exception();

    public Task StopAsync(CancellationToken cancellationToken) =>
        Task.CompletedTask;
}

Expected behavior

When an exception occurs within the StartAsync of IHostedService, whether or not method has await, the exception should be logged to the ILogger

Actual behavior

When an exception occurs in the StartAsync without calling await, the exception is not logged to the ILogger

Regression?

No response

Known Workarounds

No response

Configuration

Sdk: Microsoft.NET.Sdk.Worker
Framework: .NET 7.0.3

Other information

It is also lead to misunderstanding of BackgroundServiceExceptionBehavior.Ignore, which continue to stop host when exception occurs.

This can be relative to #67146

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Apr 10, 2023
@ghost
Copy link

ghost commented Apr 10, 2023

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

Issue Details

Description

When an exception occures in IHostedService without calling await, it is not logged to the ILogger.
This make it difficult to identify the reason for the application shutdown when writing logs to an external service.

Reproduction Steps

IHost host = Host.CreateDefaultBuilder(args)
    .ConfigureServices(services =>
    {
        services.AddHostedService<Worker>();
    })
    .Build();

await host.RunAsync();

public class Worker : IHostedService
{
    public Task StartAsync(CancellationToken cancellationToken) =>
        throw new Exception();

    public Task StopAsync(CancellationToken cancellationToken) =>
        Task.CompletedTask;
}

Expected behavior

When an exception occurs within the StartAsync of IHostedService, whether or not method has await, the exception should be logged to the ILogger

Actual behavior

When an exception occurs in the StartAsync without calling await, the exception is not logged to the ILogger

Regression?

No response

Known Workarounds

No response

Configuration

Sdk: Microsoft.NET.Sdk.Worker
Framework: .NET 7.0.3

Other information

It is also lead to misunderstanding of BackgroundServiceExceptionBehavior.Ignore, which continue to stop host when exception occurs.

This can be relative to #67146

Author: Tripletri
Assignees: -
Labels:

untriaged, area-Extensions-Hosting

Milestone: -

@buyaa-n
Copy link
Contributor

buyaa-n commented Jun 1, 2023

It looks we did not had any exception hanlding for running hosted services:

foreach (IHostedService hostedService in _hostedServices)
{
// Fire IHostedService.Start
await hostedService.StartAsync(combinedCancellationToken).ConfigureAwait(false);
if (hostedService is BackgroundService backgroundService)
{
_ = TryExecuteBackgroundServiceAsync(backgroundService);
}
}

This is updated recently with #84048 and #85191:

List<Exception> exceptions = new List<Exception>();
if (_options.ServicesStartConcurrently)
{
List<Task> tasks = new List<Task>();
foreach (IHostedService hostedService in _hostedServices)
{
tasks.Add(Task.Run(() => StartAndTryToExecuteAsync(hostedService, combinedCancellationToken), combinedCancellationToken));
}
Task groupedTasks = Task.WhenAll(tasks);
try
{
await groupedTasks.ConfigureAwait(false);
}
catch (Exception ex)
{
exceptions.AddRange(groupedTasks.Exception?.InnerExceptions ?? new[] { ex }.AsEnumerable());
}
}
else
{
foreach (IHostedService hostedService in _hostedServices)
{
try
{
// Fire IHostedService.Start
await StartAndTryToExecuteAsync(hostedService, combinedCancellationToken).ConfigureAwait(false);
}
catch (Exception ex)
{
exceptions.Add(ex);
break;
}
}
}
if (exceptions.Count > 0)
{
if (exceptions.Count == 1)
{
// Rethrow if it's a single error
Exception singleException = exceptions[0];
_logger.HostedServiceStartupFaulted(singleException);
ExceptionDispatchInfo.Capture(singleException).Throw();
}
else
{
var ex = new AggregateException("One or more hosted services failed to start.", exceptions);
_logger.HostedServiceStartupFaulted(ex);
throw ex;
}
}

So, the issues you are seeing might have already fixed. The repro you provided doesn't have a logger and the await that causing the logging work, so I cannot check. @Tripletri please tryout your code with the .NET 8 preview 5 bits and let us know.

@buyaa-n buyaa-n added this to the Future milestone Jun 1, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jun 1, 2023
@buyaa-n buyaa-n added the needs-author-action An issue or pull request that requires more info or actions from the author. label Jun 1, 2023
@ghost
Copy link

ghost commented Jun 1, 2023

This issue has been marked needs-author-action and may be missing some important information.

@Tripletri
Copy link
Author

Yes, the issue is fixed in 8.0.100-preview.5.23301.12. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators Jul 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Extensions-Hosting needs-author-action An issue or pull request that requires more info or actions from the author.
Projects
None yet
Development

No branches or pull requests

2 participants