Align host behavior with respect to exceptions (#7657)

- Don't catch errors in IHostedService.StartAsync
- Only catch errors when executing StopAsync but rethrow an aggregate
- Updated the tests

src/Hosting/Hosting/src/Internal/HostedServiceExecutor.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -21,31 +21,17 @@ namespace Microsoft.AspNetCore.Hosting.Internal
             _services = services;
         }
 
-        public async Task StartAsync(CancellationToken token)
+        public Task StartAsync(CancellationToken token)
         {
-            try
+            return ExecuteAsync(service => service.StartAsync(token));
-            {
-                await ExecuteAsync(service => service.StartAsync(token));
-            }
-            catch (Exception ex)
-            {
-                _logger.ApplicationError(LoggerEventIds.HostedServiceStartException, "An error occurred starting the application", ex);
-            }
         }
 
-        public async Task StopAsync(CancellationToken token)
+        public Task StopAsync(CancellationToken token)
         {
-            try
+            return ExecuteAsync(service => service.StopAsync(token), throwOnFirstFailure: false);
-            {
-                await ExecuteAsync(service => service.StopAsync(token));
-            }
-            catch (Exception ex)
-            {
-                _logger.ApplicationError(LoggerEventIds.HostedServiceStopException, "An error occurred stopping the application", ex);
-            }
         }
 
-        private async Task ExecuteAsync(Func<IHostedService, Task> callback)
+        private async Task ExecuteAsync(Func<IHostedService, Task> callback, bool throwOnFirstFailure = true)
         {
             List<Exception> exceptions = null;
 
@@ -57,6 +43,11 @@ namespace Microsoft.AspNetCore.Hosting.Internal
                 }
                 catch (Exception ex)
                 {
+                    if (throwOnFirstFailure)
+                    {
+                        throw;
+                    }
+
                     if (exceptions == null)
                     {
                         exceptions = new List<Exception>();

src/Hosting/Hosting/test/WebHostBuilderTests.cs

@@ -112,7 +112,7 @@ namespace Microsoft.AspNetCore.Hosting
                 await AssertResponseContains(server.RequestDelegate, "Exception from constructor");
             }
         }
-        
+
         [Theory]
         [MemberData(nameof(DefaultWebHostBuildersWithConfig))]
         public async Task StartupConfigureServicesThrows_Fallback(IWebHostBuilder builder)
@@ -967,7 +967,8 @@ namespace Microsoft.AspNetCore.Hosting
         {
             builder = builder
                 .CaptureStartupErrors(false)
-                .ConfigureAppConfiguration((context, configurationBuilder) => {
+                .ConfigureAppConfiguration((context, configurationBuilder) =>
+                {
                     configurationBuilder.AddInMemoryCollection(
                         new[]
                         {
@@ -1204,6 +1205,51 @@ namespace Microsoft.AspNetCore.Hosting
             }
         }
 
+        [Theory]
+        [MemberData(nameof(DefaultWebHostBuilders))]
+        public async Task ThrowingFromHostedServiceFailsStartAsync(IWebHostBuilder builder)
+        {
+            builder.Configure(app => { })
+                   .ConfigureServices(services =>
+                   {
+                       services.AddHostedService<ThrowingHostedService>();
+                   })
+                   .UseServer(new TestServer());
+
+            var host = builder.Build();
+            var startEx = await Assert.ThrowsAsync<InvalidOperationException>(() => host.StartAsync());
+            Assert.Equal("Hosted Service throws in StartAsync", startEx.Message);
+            var stopEx = await Assert.ThrowsAsync<AggregateException>(() => host.StopAsync());
+            Assert.Single(stopEx.InnerExceptions);
+            Assert.Equal("Hosted Service throws in StopAsync", stopEx.InnerExceptions[0].Message);
+        }
+
+        [Theory]
+        [MemberData(nameof(DefaultWebHostBuilders))]
+        public async Task ThrowingFromHostedServiceStopsOtherHostedServicesFromRunningStartAsync(IWebHostBuilder builder)
+        {
+            builder.Configure(app => { })
+                   .ConfigureServices(services =>
+                   {
+                       services.AddHostedService<ThrowingHostedService>();
+                       services.AddHostedService<NonThrowingHostedService>();
+                   })
+                   .UseServer(new TestServer());
+
+            var host = builder.Build();
+            var service = host.Services.GetServices<IHostedService>().OfType<NonThrowingHostedService>().First();
+            var startEx = await Assert.ThrowsAsync<InvalidOperationException>(() => host.StartAsync());
+            Assert.Equal("Hosted Service throws in StartAsync", startEx.Message);
+
+            var stopEx = await Assert.ThrowsAsync<AggregateException>(() => host.StopAsync());
+            Assert.Single(stopEx.InnerExceptions);
+            Assert.Equal("Hosted Service throws in StopAsync", stopEx.InnerExceptions[0].Message);
+
+            // This service is never constructed
+            Assert.False(service.StartCalled);
+            Assert.True(service.StopCalled);
+        }
+
         private static void StaticConfigureMethod(IApplicationBuilder app) { }
 
         private IWebHostBuilder CreateWebHostBuilder()
@@ -1257,6 +1303,37 @@ namespace Microsoft.AspNetCore.Hosting
             Assert.Contains(expectedText, bodyText);
         }
 
+        private class ThrowingHostedService : IHostedService
+        {
+            public Task StartAsync(CancellationToken cancellationToken)
+            {
+                throw new InvalidOperationException("Hosted Service throws in StartAsync");
+            }
+
+            public Task StopAsync(CancellationToken cancellationToken)
+            {
+                throw new NotImplementedException("Hosted Service throws in StopAsync");
+            }
+        }
+
+        private class NonThrowingHostedService : IHostedService
+        {
+            public bool StartCalled { get; set; }
+            public bool StopCalled { get; set; }
+
+            public Task StartAsync(CancellationToken cancellationToken)
+            {
+                StartCalled = true;
+                return Task.CompletedTask;
+            }
+
+            public Task StopAsync(CancellationToken cancellationToken)
+            {
+                StopCalled = true;
+                return Task.CompletedTask;
+            }
+        }
+
         private class MyStartupFilter : IStartupFilter
         {
             public bool Executed { get; set; }

src/Hosting/Hosting/test/WebHostTests.cs

@@ -458,26 +458,26 @@ namespace Microsoft.AspNetCore.Hosting
         }
 
         [Fact]
-        public async Task WebHostNotifiesAllIApplicationLifetimeEventsCallbacksEvenIfTheyThrow()
+        public async Task WebHostDoesNotNotifyAllIApplicationLifetimeEventsCallbacksIfTheyThrow()
         {
-            bool[] events1 = null;
+            bool[] hostedSeviceCalls1 = null;
-            bool[] events2 = null;
+            bool[] hostedServiceCalls2 = null;
 
             using (var host = CreateBuilder()
                 .UseFakeServer()
                 .ConfigureServices(services =>
                 {
-                    events1 = RegisterCallbacksThatThrow(services);
+                    hostedSeviceCalls1 = RegisterCallbacksThatThrow(services);
-                    events2 = RegisterCallbacksThatThrow(services);
+                    hostedServiceCalls2 = RegisterCallbacksThatThrow(services);
                 })
                 .Build())
             {
-                await host.StartAsync();
+                await Assert.ThrowsAsync<InvalidOperationException>(() => host.StartAsync());
-                Assert.True(events1[0]);
+                Assert.True(hostedSeviceCalls1[0]);
-                Assert.True(events2[0]);
+                Assert.False(hostedServiceCalls2[0]);
                 host.Dispose();
-                Assert.True(events1[1]);
+                Assert.True(hostedSeviceCalls1[1]);
-                Assert.True(events2[1]);
+                Assert.True(hostedServiceCalls2[1]);
             }
         }
 
@@ -667,17 +667,17 @@ namespace Microsoft.AspNetCore.Hosting
         }
 
         [Fact]
-        public async Task WebHostNotifiesAllIHostedServicesAndIApplicationLifetimeCallbacksEvenIfTheyThrow()
+        public async Task WebHostDoesNotNotifyAllIHostedServicesAndIApplicationLifetimeCallbacksIfTheyThrow()
         {
-            bool[] events1 = null;
+            bool[] hostedServiceCalls1 = null;
-            bool[] events2 = null;
+            bool[] hostedServiceCalls2 = null;
 
             using (var host = CreateBuilder()
                 .UseFakeServer()
                 .ConfigureServices(services =>
                 {
-                    events1 = RegisterCallbacksThatThrow(services);
+                    hostedServiceCalls1 = RegisterCallbacksThatThrow(services);
-                    events2 = RegisterCallbacksThatThrow(services);
+                    hostedServiceCalls2 = RegisterCallbacksThatThrow(services);
                 })
                 .Build())
             {
@@ -690,14 +690,14 @@ namespace Microsoft.AspNetCore.Hosting
                 var started2 = RegisterCallbacksThatThrow(applicationLifetime2.ApplicationStarted);
                 var stopping2 = RegisterCallbacksThatThrow(applicationLifetime2.ApplicationStopping);
 
-                await host.StartAsync();
+                await Assert.ThrowsAsync<InvalidOperationException>(() => host.StartAsync());
-                Assert.True(events1[0]);
+                Assert.True(hostedServiceCalls1[0]);
-                Assert.True(events2[0]);
+                Assert.False(hostedServiceCalls2[0]);
                 Assert.True(started.All(s => s));
                 Assert.True(started2.All(s => s));
                 host.Dispose();
-                Assert.True(events1[1]);
+                Assert.True(hostedServiceCalls1[1]);
-                Assert.True(events2[1]);
+                Assert.True(hostedServiceCalls2[1]);
                 Assert.True(stopping.All(s => s));
                 Assert.True(stopping2.All(s => s));
             }