Add the ability to stop the Host when one of its BackgroundService instances errors

[IEvangelist]

• Created BackgroundServiceExceptionBehavior enum.
• Added HostOptions.BackgroundServiceExceptionBehavior property.
  • Defaults to StopHost, which was not the original behavior.
• Added corresponding logic in Host
  • Renamed HandleBackgroundException to TryStartBackgroundServiceAsync as it is more conditional and appended the "Async" suffix as it is Task returning
  • Moved the invocation of StartAsync to the TryStartBackgroundServiceAsync to ensure that situations where it immediately throws are correctly handled.
• Added unit tests.
  • Cover default behavior Ignore explicitly
  • Cover StopHost which calls IApplicationLifetime.StopApplication(), thus triggering ApplicationStopping.

Fixes #43637

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

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.

[ghost]

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

---

• Created BackgroundServiceExceptionBehavior enum.
• Added HostOptions.BackgroundServiceExceptionBehavior.
  • Defaults to Ignore, which is the current behavior.
• Added corresponding logic in Host
  • Renamed HandleBackgroundException to TryStartBackgroundServiceAsync as it is more conditional and appended the "Async" suffix as it is Task returning
  • Moved the invocation of StartAsync to the TryStartBackgroundServiceAsync to ensure that situations where it immediately throws are correctly handled.
• Added unit tests.
  • Cover default behavior Ignore explicitly
  • Cover StopHost which calls Environment.FailFast -- needed to use RemoteExecutor (thanks @eerhardt for the pointer)

Fixes #43637

Author:	IEvangelist
Assignees:	-
Labels:	area-Extensions-Hosting, new-api-needs-documentation
Milestone:	-

[eerhardt]

Looks like the new test is failling.

[eerhardt]

Assuming CI passes clean, I think this is ready to be merged.

Thanks for the good work here, @IEvangelist.

[davidfowl]

The one question I have is the behavior around starting. Before if you threw an exception on start it would fail starting the host but it never called StopApplication since the application never started in the first place and the exception bubbled to the the host.StartAsync. What's the behavior here? Is it still the same?

[IEvangelist]

The one question I have is the behavior around starting. Before if you threw an exception on start it would fail starting the host but it never called StopApplication since the application never started in the first place and the exception bubbled to the the host.StartAsync. What's the behavior here? Is it still the same?

@davidfowl - This does introduce a change in behavior:

• When an exception is thrown from a BackgroundService it is either ignored (logged), or a call to StopApplication is made
  • The Exception is not bubbled up to host.StartAsync.
• If an exception is thrown from an IHostedService it bubbles as it always has, in other words - no change in behavior here

We will need to write up a "breaking change" doc, per @eerhardt. With these changes, by default, we'll stop the host.

Before these changes, if the host started a BackgroundService that threw immediately on its StartAsync call - it would bubble up, just like with IHostedService, and it was only "handled" with the call to await backgroundService.ExecuteTask. Hope that makes sense.

[davidfowl]

The Exception is not bubbled up to host.StartAsync.

I think that behavior is wrong. We should revert it.

[IEvangelist]

The Exception is not bubbled up to host.StartAsync.

I think that behavior is wrong. We should revert it.

@davidfowl - In looking at this again - an Exception thrown from a BackgroundService was only ever bubbled up to host.StartAsync when it was thrown from:

// Fire IHostedService.Start
await hostedService.StartAsync(combinedCancellationToken).ConfigureAwait(false);

Otherwise it was just logged in HandleBackgroundException. I believe that this was probably extremely rare, and I wouldn't be surprised if most errors occurred in:

await backgroundService.ExecuteTask.ConfigureAwait(false);

Since it is possible for an error to occur on either call, I intentionally grouped these two together to capture both possible background service failure scenarios. Are you suggesting that we throw from within the TryStartBackgroundServiceAsync? If so, why bother calling StopApplication?

I'm still a bit confused on why we wouldn't call StopAsync in the catch clause in the TryStartBackgroundServiceAsync method.

Update in chatting this over with @eerhardt I have a better understanding now. Updating this now.

[eerhardt]

LGTM. @davidfowl - any thoughts?

[davidfowl]

Very nice!

[eerhardt]

Test failures seem unrelated:

C:\h\w\A918093D\w\A6DD0962\e>"C:\h\w\A918093D\p\dotnet.exe" exec --runtimeconfig System.Linq.Expressions.Tests.runtimeconfig.json --depsfile System.Linq.Expressions.Tests.deps.json xunit.console.dll System.Linq.Expressions.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing  
  Discovering: System.Linq.Expressions.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Linq.Expressions.Tests (found 5645 of 5657 test cases)
  Starting:    System.Linq.Expressions.Tests (parallel test collections = on, max threads = 2)
    System.Linq.Expressions.Tests.ConvertTests.ConvertFloatToNullableULongTest(useInterpreter: False) [FAIL]
      Assert.Equal() Failure
      Expected: 0
      Actual:   9223372036854775808
      Stack Trace:
        /_/src/libraries/System.Linq.Expressions/tests/Convert/ConvertTests.cs(11459,0): at System.Linq.Expressions.Tests.ConvertTests.VerifyFloatToNullableULong(Single value, Boolean useInterpreter)
        /_/src/libraries/System.Linq.Expressions/tests/Convert/ConvertTests.cs(3128,0): at System.Linq.Expressions.Tests.ConvertTests.ConvertFloatToNullableULongTest(Boolean useInterpreter)
    System.Linq.Expressions.Tests.ConvertTests.ConvertDoubleToNullableULongTest(useInterpreter: False) [FAIL]
      Assert.Equal() Failure
      Expected: 0
      Actual:   9223372036854775808
      Stack Trace:
        /_/src/libraries/System.Linq.Expressions/tests/Convert/ConvertTests.cs(9533,0): at System.Linq.Expressions.Tests.ConvertTests.VerifyDoubleToNullableULong(Double value, Boolean useInterpreter)
        /_/src/libraries/System.Linq.Expressions/tests/Convert/ConvertTests.cs(1686,0): at System.Linq.Expressions.Tests.ConvertTests.ConvertDoubleToNullableULongTest(Boolean useInterpreter)
    System.Linq.Expressions.Tests.ConvertTests.ConvertNullableDoubleToNullableULongTest(useInterpreter: False) [FAIL]
      Assert.Equal() Failure
      Expected: 0
      Actual:   9223372036854775808
      Stack Trace:
        /_/src/libraries/System.Linq.Expressions/tests/Convert/ConvertTests.cs(9904,0): at System.Linq.Expressions.Tests.ConvertTests.VerifyNullableDoubleToNullableULong(Nullable`1 value, Boolean useInterpreter)
        /_/src/libraries/System.Linq.Expressions/tests/Convert/ConvertTests.cs(1938,0): at System.Linq.Expressions.Tests.ConvertTests.ConvertNullableDoubleToNullableULongTest(Boolean useInterpreter)
    System.Linq.Expressions.Tests.ConvertTests.ConvertNullableFloatToNullableULongTest(useInterpreter: False) [FAIL]
      Assert.Equal() Failure
      Expected: 0
      Actual:   9223372036854775808
      Stack Trace:
        /_/src/libraries/System.Linq.Expressions/tests/Convert/ConvertTests.cs(11830,0): at System.Linq.Expressions.Tests.ConvertTests.VerifyNullableFloatToNullableULong(Nullable`1 value, Boolean useInterpreter)
        /_/src/libraries/System.Linq.Expressions/tests/Convert/ConvertTests.cs(3380,0): at System.Linq.Expressions.Tests.ConvertTests.ConvertNullableFloatToNullableULongTest(Boolean useInterpreter)
  Finished:    System.Linq.Expressions.Tests
=== TEST EXECUTION SUMMARY ===
   System.Linq.Expressions.Tests  Total: 35230, Errors: 0, Failed: 4, Skipped: 0, Time: 29.861s

if this change broke System.Linq.Expressions, I'm going to quit programming because computers don't make sense any more.

This smells like a coreclr break.

[eerhardt]

Logged #51346 for the test failures.

[eerhardt]

Libraries test failures are #51346
Installer test failures are #51372