Propagate BackgroundService exceptions from the host#124863
Propagate BackgroundService exceptions from the host#124863svick wants to merge 3 commits intodotnet:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes issue #67146 by ensuring that exceptions thrown asynchronously by BackgroundServices propagate to the caller and result in a non-zero exit code, matching the behavior when exceptions are thrown synchronously.
Changes:
- Added exception tracking and propagation for background services in StopHost behavior mode
- Replaced trivial enum tests with comprehensive functional tests covering various exception scenarios
- Added logging for exception propagation
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs | Added _backgroundServiceExceptions list to track background service exceptions and propagate them at the end of StopAsync(), ensuring non-zero exit codes |
| src/libraries/Microsoft.Extensions.Hosting/src/Internal/HostingLoggerExtensions.cs | Added BackgroundServiceExceptionsPropagating logging method and fixed signature of StoppedWithException to require non-null exception |
| src/libraries/Microsoft.Extensions.Hosting/src/Internal/LoggerEventIds.cs | Added BackgroundServiceExceptionsPropagating event ID |
| src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/BackgroundServiceExceptionTests.cs | Added comprehensive tests for synchronous/asynchronous exceptions, multiple exceptions, and different exception behaviors |
| src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/BackgroundServiceExceptionBehaviorTests.cs | Removed trivial enum value tests in favor of functional tests |
Comments suppressed due to low confidence (6)
src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs:146
- The comment change from "Call" to "Cancel" is inaccurate. While
NotifyStarted()internally cancels a CancellationTokenSource to signal the ApplicationStarted event, the comment should describe the semantic action (calling/triggering the notification), not the internal implementation detail. The original wording "Call IHostApplicationLifetime.Started" better describes what the code is doing at this level of abstraction.
// Cancel IHostApplicationLifetime.Started
src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs:239
- The comment change from "Call" to "Cancel" is inaccurate. While
StopApplication()internally cancels a CancellationTokenSource to signal the ApplicationStopping event, the comment should describe the semantic action (calling/triggering the event), not the internal implementation detail. The original wording "Call IHostApplicationLifetime.ApplicationStopping" better describes what the code is doing at this level of abstraction.
// Cancel IHostApplicationLifetime.ApplicationStopping.
src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs:259
- The comment change from "Call" to "Cancel" is inaccurate. While
StopApplication()internally cancels a CancellationTokenSource to signal the ApplicationStopping event, the comment should describe the semantic action (calling/triggering the event), not the internal implementation detail. The original wording "Call IHostApplicationLifetime.ApplicationStopping" better describes what the code is doing at this level of abstraction.
// Cancel IHostApplicationLifetime.ApplicationStopping.
src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs:275
- The comment change from "Call" to "Cancel" is inaccurate. While
NotifyStopped()internally cancels a CancellationTokenSource to signal the ApplicationStopped event, the comment should describe the semantic action (calling/triggering the notification), not the internal implementation detail. The original wording "Call IHostApplicationLifetime.Stopped" better describes what the code is doing at this level of abstraction.
// Cancel IHostApplicationLifetime.Stopped
src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/BackgroundServiceExceptionTests.cs:82
- The hardcoded 500ms delay creates a race condition and could lead to flaky tests. Consider using ApplicationStopping.WaitHandle.WaitOne (as seen in HostTests.cs line 710) or TaskCompletionSource patterns (as seen in HostTests.cs lines 713-758) for more reliable synchronization. These approaches ensure the background service has actually failed before calling StopAsync, rather than relying on timing assumptions.
await Task.Delay(TimeSpan.FromMilliseconds(500));
src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/BackgroundServiceExceptionTests.cs:163
- The hardcoded 2-second delay is longer than necessary and slows down test execution. Consider using ApplicationStopping.WaitHandle.WaitOne or TaskCompletionSource patterns for more efficient and reliable synchronization, as demonstrated in existing tests in HostTests.cs.
await Task.Delay(TimeSpan.FromSeconds(2));
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs:146
- The comment change from "Call" to "Cancel" appears to be unrelated to the main purpose of this PR (exception propagation). While technically more accurate (the implementation calls Cancel() on CancellationTokenSources), this type of refactoring should ideally be separated from functional changes. Consider whether these comment changes should be part of a separate PR or if they're necessary for this change.
// Cancel IHostApplicationLifetime.Started
src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/BackgroundServiceExceptionTests.cs:82
- The test relies on fixed time delays (200ms) to ensure the background service has failed before calling StopAsync. This could lead to flaky tests on slower systems or under high load. Consider using synchronization primitives like TaskCompletionSource or ManualResetEventSlim to reliably wait for the service to fail, similar to patterns used in BackgroundServiceTests.cs (e.g., WaitForExecuteTask).
await Task.Delay(TimeSpan.FromMilliseconds(200));
src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/BackgroundServiceExceptionTests.cs:163
- Similar to the previous test, this test uses a fixed 200ms delay which could cause flakiness on slower systems. Consider using synchronization primitives to ensure the background service has completed its failure path before calling StopAsync.
await Task.Delay(TimeSpan.FromMilliseconds(200));
src/libraries/Microsoft.Extensions.Hosting/src/Internal/HostingLoggerExtensions.cs:72
- The nullable annotation has been removed from the Exception parameter. However, in the current codebase, this method is only called when an exception exists (lines 297 and 303 in Host.cs), so the change is safe. Consider verifying that all call sites indeed pass non-null exceptions to ensure this change doesn't introduce any issues.
public static void StoppedWithException(this ILogger logger, Exception ex)
src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs:328
- The _backgroundServiceExceptions list is checked and thrown at the end of StopAsync, but there's a potential race condition: if a background service exception is added to the list after line 289 sets _hostStopped = true but before the exception propagation check at lines 311-328, the exception would still be thrown. However, if StopAsync is somehow called again, it would re-throw the same exceptions because the list is never cleared. Consider clearing the exception list after throwing to prevent potential issues with multiple StopAsync calls, or document that StopAsync should only be called once.
// If background services faulted and caused the host to stop, rethrow the exceptions
// so they propagate and cause a non-zero exit code.
lock (_backgroundServiceExceptions)
{
if (_backgroundServiceExceptions.Count == 1)
{
_logger.BackgroundServiceExceptionsPropagating(_backgroundServiceExceptions[0].SourceException);
_backgroundServiceExceptions[0].Throw();
}
else if (_backgroundServiceExceptions.Count > 1)
{
var aggregateException = new AggregateException(
"One or more background services threw an exception.",
_backgroundServiceExceptions.Select(edi => edi.SourceException));
_logger.BackgroundServiceExceptionsPropagating(aggregateException);
throw aggregateException;
}
}
Fixes #67146 by throwing from
IHost.RunAsync/StopAsyncwhen aBackgroundServicefails with an exception andBackgroundServiceExceptionBehavior.StopHostis set.I think this should be documented as a breaking change, since users could be relying on having a throwing
BackgroundServicethat stops the application but returns with success exit code.