Fix apphost crashing when dependencies fail. (#6101)

* Fix apphost crashing when dependencies fail. * Revert sample. * Update playground/Redis/Redis.AppHost/Program.cs * Ensure resource dependencies can't kill the apphost when user code throws. * Requires docker. --------- Co-authored-by: David Fowler <[email protected]>
dotnet · Oct 4, 2024 · c71fab2 · c71fab2
1 parent 505c05e
commit c71fab2
Show file tree

Hide file tree

Showing 2 changed files with 65 additions and 27 deletions.
diff --git a/src/Aspire.Hosting/Dcp/ApplicationExecutor.cs b/src/Aspire.Hosting/Dcp/ApplicationExecutor.cs
@@ -1188,41 +1188,55 @@ private Task CreateExecutablesAsync(IEnumerable<AppResource> executableResources
 
             async Task CreateResourceExecutablesAsyncCore(IResource resource, IEnumerable<AppResource> executables, CancellationToken cancellationToken)
             {
-                var logger = loggerService.GetLogger(resource);
+                var resourceLogger = loggerService.GetLogger(resource);
 
-                await notificationService.PublishUpdateAsync(resource, s => s with
+                try
                 {
-                    ResourceType = resource is ProjectResource ? KnownResourceTypes.Project : KnownResourceTypes.Executable,
-                    Properties = [],
-                    State = "Starting"
-                })
-                .ConfigureAwait(false);
+                    await notificationService.PublishUpdateAsync(resource, s => s with
+                    {
+                        ResourceType = resource is ProjectResource ? KnownResourceTypes.Project : KnownResourceTypes.Executable,
+                        Properties = [],
+                        State = "Starting"
+                    })
+                    .ConfigureAwait(false);
 
-                await PublishConnectionStringAvailableEvent(resource, cancellationToken).ConfigureAwait(false);
+                    await PublishConnectionStringAvailableEvent(resource, cancellationToken).ConfigureAwait(false);
 
-                var beforeResourceStartedEvent = new BeforeResourceStartedEvent(resource, serviceProvider);
-                await eventing.PublishAsync(beforeResourceStartedEvent, cancellationToken).ConfigureAwait(false);
+                    var beforeResourceStartedEvent = new BeforeResourceStartedEvent(resource, serviceProvider);
+                    await eventing.PublishAsync(beforeResourceStartedEvent, cancellationToken).ConfigureAwait(false);
 
-                foreach (var cr in executables)
-                {
-                    try
+                    foreach (var er in executables)
                     {
-                        await CreateExecutableAsync(cr, logger, cancellationToken).ConfigureAwait(false);
-                    }
-                    catch (FailedToApplyEnvironmentException)
-                    {
-                        // For this exception we don't want the noise of the stack trace, we've already
-                        // provided more detail where we detected the issue (e.g. envvar name). To get
-                        // more diagnostic information reduce logging level for DCP log category to Debug.
-                        await notificationService.PublishUpdateAsync(cr.ModelResource, cr.DcpResource.Metadata.Name, s => s with { State = "FailedToStart" }).ConfigureAwait(false);
-                    }
-                    catch (Exception ex)
-                    {
-                        logger.LogError(ex, "Failed to create resource {ResourceName}", cr.ModelResource.Name);
-
-                        await notificationService.PublishUpdateAsync(cr.ModelResource, cr.DcpResource.Metadata.Name, s => s with { State = "FailedToStart" }).ConfigureAwait(false);
+                        try
+                        {
+                            await CreateExecutableAsync(er, resourceLogger, cancellationToken).ConfigureAwait(false);
+                        }
+                        catch (FailedToApplyEnvironmentException)
+                        {
+                            // For this exception we don't want the noise of the stack trace, we've already
+                            // provided more detail where we detected the issue (e.g. envvar name). To get
+                            // more diagnostic information reduce logging level for DCP log category to Debug.
+                            await notificationService.PublishUpdateAsync(er.ModelResource, er.DcpResource.Metadata.Name, s => s with { State = "FailedToStart" }).ConfigureAwait(false);
+                        }
+                        catch (Exception ex)
+                        {
+                            // The purpose of this catch block is to ensure that if an individual executable resource fails
+                            // to start that it doesn't tear down the entire app host AND that we route the error to the
+                            // appropriate replica.
+                            resourceLogger.LogError(ex, "Failed to create resource {ResourceName}", er.ModelResource.Name);
+                            await notificationService.PublishUpdateAsync(er.ModelResource, er.DcpResource.Metadata.Name, s => s with { State = "FailedToStart" }).ConfigureAwait(false);
+                        }
                     }
                 }
+                catch (Exception ex)
+                {
+                    // The purpose of this catch block is to ensure that if an error processing the overall
+                    // configuration of the executable resource files. This is different to the exception handling
+                    // block above because at this tage of processing we don't necessarily have any replicas
+                    // yet. For example if a dependency fails to start.
+                    resourceLogger.LogError(ex, "Failed to create resource {ResourceName}", resource.Name);
+                    await notificationService.PublishUpdateAsync(resource, s => s with { State = "FailedToStart" }).ConfigureAwait(false);
+                }
             }
 
             var tasks = new List<Task>();

diff --git a/tests/Aspire.Hosting.Tests/WaitForTests.cs b/tests/Aspire.Hosting.Tests/WaitForTests.cs
@@ -11,6 +11,30 @@ namespace Aspire.Hosting.Tests;
 
 public class WaitForTests(ITestOutputHelper testOutputHelper)
 {
+    [Fact]
+    [RequiresDocker]
+    public async Task ResourceThatFailsToStartDueToExceptionDoesNotCauseStartAsyncToThrow()
+    {
+        using var builder = TestDistributedApplicationBuilder.Create().WithTestAndResourceLogging(testOutputHelper);
+        var throwingResource = builder.AddContainer("throwingresource", "doesnotmatter")
+                              .WithEnvironment(ctx => throw new InvalidOperationException("BOOM!"));
+        var dependingContainerResource = builder.AddContainer("dependingcontainerresource", "doesnotmatter")
+                                       .WaitFor(throwingResource);
+        var dependingExecutableResource = builder.AddExecutable("dependingexecutableresource", "doesnotmatter", "alsodoesntmatter")
+                                       .WaitFor(throwingResource);
+
+        var abortCts = new CancellationTokenSource(TimeSpan.FromSeconds(60));
+        using var app = builder.Build();
+        await app.StartAsync(abortCts.Token);
+
+        var rns = app.Services.GetRequiredService<ResourceNotificationService>();
+        await rns.WaitForResourceAsync(throwingResource.Resource.Name, KnownResourceStates.FailedToStart, abortCts.Token);
+        await rns.WaitForResourceAsync(dependingContainerResource.Resource.Name, KnownResourceStates.FailedToStart, abortCts.Token);
+        await rns.WaitForResourceAsync(dependingExecutableResource.Resource.Name, KnownResourceStates.FailedToStart, abortCts.Token);
+
+        await app.StopAsync(abortCts.Token);
+    }
+
     [Fact]
     public void ResourceCannotWaitForItself()
     {