microsoft / microsoft/aspire

`StartAsync` swallows `OperationCancelledException`s

Open
#13,728 0 comments 0 reactions 0 assignees View on GitHub
area-app-model area-app-testing
Dominant language
C#
Stars
6.3k
Forks
991
Avg merge
2d 15h
Merged PRs (30d)
196

Description

### Is there an existing issue for this?

- [x] I have searched the existing issues

### Describe the bug

If `DistributedApplication.StartAsync()` blocks, and you try to cancel it with a cancellation token, then the StartAsync cancels itself, but it swallows the cancellation exception.

This is particularly problematic under `DistributedApplicationTestingBuilder` scenarios as the app host at this point would be in an undefined state, without DCP having been fully initialised.

### Expected Behavior

If `StartAsync()` is cancelled without completing successfully, it must not swallow the cancellation.

### Steps To Reproduce

The following test case reproduces the issue. The infinite delay in `OnBeforeResourceStarted` blocks `StartAsync`, to allow the cancellation token to be hit. The `StartAsync()` call runs for the full length of the timeout, only finishing after the token is cancelled (as verified by `Assert.True(cts.Token.IsCancellationRequested);`), and yet StartAsync didn't throw the cancellation exception.

```cs
[Fact]
public async Task CancellationShouldNotBeSwallowed()
{
var cts = new CancellationTokenSource();
cts.Token.Register(() => Console.WriteLine("Token is cancelled"));

await using var builder = DistributedApplicationTestingBuilder.Create();
builder.AddContainer("nginx", "nginx")
.OnBeforeResourceStarted((_, _, _) => Task.Delay(Timeout.InfiniteTimeSpan));

await using var app = await builder.BuildAsync(cts.Token);

await Assert.ThrowsAsync(async () =>
{
cts.CancelAfter(TimeSpan.FromSeconds(10));
await app.StartAsync(cts.Token);
Assert.True(cts.Token.IsCancellationRequested);
});
}
```

### Exceptions (if any)

_No response_

### .NET Version info

_No response_

### Anything else?

The issue specifically seems to be caused by the following, which catches the operation cancelled exception, but then swallows it.

https://github.com/dotnet/aspire/blob/bed20b5c808c4d2cc7dea7a826a6f28d97195aba/src/Aspire.Hosting/Dcp/DcpExecutor.cs#L212-L216

This seems to have been introduced in #8886 as a solution to some rogue cancellation exceptions when `ctrl+c`ing the app host. This fix needs to be narrowed to specifically account for a console termination error, and not all cancellation tokens.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.