From d7814423ff3fee8ccfe39c6075b05cccf067bf3b Mon Sep 17 00:00:00 2001
From: Steve Harter <steveharter@users.noreply.github.com>
Date: Tue, 8 Aug 2023 14:24:05 -0500
Subject: [PATCH] Abort startup if Starting, Start or Started fails

---
 .../src/Internal/Host.cs                      | 16 +++--
 .../tests/UnitTests/LifecycleTests.Start.cs   | 65 ++++++++++++++++---
 .../tests/UnitTests/LifecycleTests.Stop.cs    |  2 +-
 .../tests/UnitTests/LifecycleTests.cs         | 24 +++++--
 4 files changed, 86 insertions(+), 21 deletions(-)

diff --git a/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs b/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs
index 8a242b8adf5580..3b0f4d8e30d3ec 100644
--- a/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs
+++ b/src/libraries/Microsoft.Extensions.Hosting/src/Internal/Host.cs
@@ -62,8 +62,8 @@ public Host(IServiceProvider services,
 
         /// <summary>
         /// Order:
-        ///  IHostLifetime.WaitForStartAsync (can abort chain)
-        ///  Services.GetService{IStartupValidator}().Validate() (can abort chain)
+        ///  IHostLifetime.WaitForStartAsync
+        ///  Services.GetService{IStartupValidator}().Validate()
         ///  IHostedLifecycleService.StartingAsync
         ///  IHostedService.Start
         ///  IHostedLifecycleService.StartedAsync
@@ -123,11 +123,11 @@ public async Task StartAsync(CancellationToken cancellationToken = default)
                     await ForeachService(_hostedLifecycleServices, token, concurrent, abortOnFirstException, exceptions,
                         (service, token) => service.StartingAsync(token)).ConfigureAwait(false);
 
-                    // We do not abort on exceptions from StartingAsync.
+                    // Exceptions in StartingAsync cause startup to be aborted.
+                    LogAndRethrow();
                 }
 
                 // Call StartAsync().
-                // We do not abort on exceptions from StartAsync.
                 await ForeachService(_hostedServices, token, concurrent, abortOnFirstException, exceptions,
                     async (service, token) =>
                     {
@@ -139,14 +139,17 @@ await ForeachService(_hostedServices, token, concurrent, abortOnFirstException,
                         }
                     }).ConfigureAwait(false);
 
+                // Exceptions in StartAsync cause startup to be aborted.
+                LogAndRethrow();
+
                 // Call StartedAsync().
-                // We do not abort on exceptions from StartedAsync.
                 if (_hostedLifecycleServices is not null)
                 {
                     await ForeachService(_hostedLifecycleServices, token, concurrent, abortOnFirstException, exceptions,
                         (service, token) => service.StartedAsync(token)).ConfigureAwait(false);
                 }
 
+                // Exceptions in StartedAsync cause startup to be aborted.
                 LogAndRethrow();
 
                 // Call IHostApplicationLifetime.Started
@@ -329,7 +332,7 @@ private static async Task ForeachService<T>(
             {
                 // The beginning synchronous portions of the implementations are run serially in registration order for
                 // performance since it is common to return Task.Completed as a noop.
-                // Any subsequent asynchronous portions are grouped together run concurrently.
+                // Any subsequent asynchronous portions are grouped together and run concurrently.
                 List<Task>? tasks = null;
 
                 foreach (T service in services)
@@ -354,6 +357,7 @@ private static async Task ForeachService<T>(
                     }
                     else
                     {
+                        // The task encountered an await; add it to a list to run concurrently.
                         tasks ??= new();
                         tasks.Add(Task.Run(() => task, token));
                     }
diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/LifecycleTests.Start.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/LifecycleTests.Start.cs
index 70cee505aadb0b..f4008f7d6b2d45 100644
--- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/LifecycleTests.Start.cs
+++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/LifecycleTests.Start.cs
@@ -313,9 +313,61 @@ public async Task StartedAsync(CancellationToken cancellationToken)
         [Theory]
         [InlineData(true)]
         [InlineData(false)]
-        public async Task StartPhasesException(bool throwAfterAsyncCall)
+        public async Task StartPhasesException_Starting(bool throwAfterAsyncCall)
         {
-            ExceptionImpl impl = new(throwAfterAsyncCall: throwAfterAsyncCall, throwOnStartup: true, throwOnShutdown: false);
+            ExceptionImpl impl = new(throwAfterAsyncCall: throwAfterAsyncCall,
+                throwOnStarting: true, throwOnStart: false, throwOnStarted: false);
+
+            var hostBuilder = CreateHostBuilder(services =>
+            {
+                services.AddHostedService((token) => impl);
+            });
+
+            using (IHost host = hostBuilder.Build())
+            {
+                Exception ex = await Assert.ThrowsAnyAsync<Exception>(async () => await host.StartAsync());
+
+                Assert.True(impl.StartingCalled);
+                Assert.False(impl.StartCalled);
+                Assert.False(impl.StartedCalled);
+
+                Assert.Contains("(ThrowOnStarting)", ex.Message);
+            }
+        }
+
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public async Task StartPhasesException_Start(bool throwAfterAsyncCall)
+        {
+            ExceptionImpl impl = new(throwAfterAsyncCall: throwAfterAsyncCall,
+                throwOnStarting: false, throwOnStart: true, throwOnStarted: false);
+
+            var hostBuilder = CreateHostBuilder(services =>
+            {
+                services.AddHostedService((token) => impl);
+            });
+
+            using (IHost host = hostBuilder.Build())
+            {
+                Exception ex = await Assert.ThrowsAnyAsync<Exception>(async () => await host.StartAsync());
+
+                Assert.True(impl.StartingCalled);
+                Assert.True(impl.StartCalled);
+                Assert.False(impl.StartedCalled);
+
+                Assert.Contains("(ThrowOnStart)", ex.Message);
+            }
+        }
+
+        [Theory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public async Task StartPhasesException_Started(bool throwAfterAsyncCall)
+        {
+            ExceptionImpl impl = new(throwAfterAsyncCall: throwAfterAsyncCall,
+                throwOnStarting: false, throwOnStart: false, throwOnStarted: true);
+
             var hostBuilder = CreateHostBuilder(services =>
             {
                 services.AddHostedService((token) => impl);
@@ -323,23 +375,20 @@ public async Task StartPhasesException(bool throwAfterAsyncCall)
 
             using (IHost host = hostBuilder.Build())
             {
-                AggregateException ex = await Assert.ThrowsAnyAsync<AggregateException>(async () => await host.StartAsync());
+                Exception ex = await Assert.ThrowsAnyAsync<Exception>(async () => await host.StartAsync());
 
                 Assert.True(impl.StartingCalled);
                 Assert.True(impl.StartCalled);
                 Assert.True(impl.StartedCalled);
 
-                Assert.Equal(3, ex.InnerExceptions.Count);
-                Assert.Contains("(ThrowOnStarting)", ex.InnerExceptions[0].Message);
-                Assert.Contains("(ThrowOnStart)", ex.InnerExceptions[1].Message);
-                Assert.Contains("(ThrowOnStarted)", ex.InnerExceptions[2].Message);
+                Assert.Contains("(ThrowOnStarted)", ex.Message);
             }
         }
 
         [Fact]
         public async Task ValidateOnStartAbortsChain()
         {
-            ExceptionImpl impl = new(throwAfterAsyncCall: true, throwOnStartup: true, throwOnShutdown: false);
+            ExceptionImpl impl = new(throwAfterAsyncCall: false, throwOnStarting: false, throwOnStart: false, throwOnStarted: false);
             var hostBuilder = CreateHostBuilder(services =>
             {
                 services.AddHostedService((token) => impl)
diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/LifecycleTests.Stop.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/LifecycleTests.Stop.cs
index 2b413d29be3b22..03e0b0124d465d 100644
--- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/LifecycleTests.Stop.cs
+++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/LifecycleTests.Stop.cs
@@ -320,7 +320,7 @@ public async Task StoppedAsync(CancellationToken cancellationToken)
         [InlineData(false)]
         public async Task StopPhasesException(bool throwAfterAsyncCall)
         {
-            ExceptionImpl impl = new(throwAfterAsyncCall: throwAfterAsyncCall, throwOnStartup: false, throwOnShutdown: true);
+            ExceptionImpl impl = new(throwAfterAsyncCall: throwAfterAsyncCall, throwOnShutdown: true);
             var hostBuilder = CreateHostBuilder(services =>
             {
                 services.AddHostedService((token) => impl);
diff --git a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/LifecycleTests.cs b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/LifecycleTests.cs
index 373913cc3de6bf..346d759ac63709 100644
--- a/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/LifecycleTests.cs
+++ b/src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/LifecycleTests.cs
@@ -180,23 +180,35 @@ private class ExceptionImpl : IHostedLifecycleService
             public bool StopCalled = false;
             public bool StoppedCalled = false;
 
-            public bool ThrowOnStartup;
+            public bool ThrowOnStarting;
+            public bool ThrowOnStart;
+            public bool ThrowOnStarted;
             public bool ThrowOnShutdown;
 
             public ExceptionImpl(
                 bool throwAfterAsyncCall,
-                bool throwOnStartup,
                 bool throwOnShutdown)
             {
                 _throwAfterAsyncCall = throwAfterAsyncCall;
-                ThrowOnStartup = throwOnStartup;
                 ThrowOnShutdown = throwOnShutdown;
             }
 
+            public ExceptionImpl(
+                bool throwAfterAsyncCall,
+                bool throwOnStarting,
+                bool throwOnStart,
+                bool throwOnStarted)
+            {
+                _throwAfterAsyncCall = throwAfterAsyncCall;
+                ThrowOnStarting = throwOnStarting;
+                ThrowOnStart = throwOnStart;
+                ThrowOnStarted = throwOnStarted;
+            }
+
             public async Task StartingAsync(CancellationToken cancellationToken)
             {
                 StartingCalled = true;
-                if (ThrowOnStartup)
+                if (ThrowOnStarting)
                 {
                     if (_throwAfterAsyncCall)
                     {
@@ -210,7 +222,7 @@ public async Task StartingAsync(CancellationToken cancellationToken)
             public async Task StartAsync(CancellationToken cancellationToken)
             {
                 StartCalled = true;
-                if (ThrowOnStartup)
+                if (ThrowOnStart)
                 {
                     if (_throwAfterAsyncCall)
                     {
@@ -224,7 +236,7 @@ public async Task StartAsync(CancellationToken cancellationToken)
             public async Task StartedAsync(CancellationToken cancellationToken)
             {
                 StartedCalled = true;
-                if (ThrowOnStartup)
+                if (ThrowOnStarted)
                 {
                     if (_throwAfterAsyncCall)
                     {
