`StartAsync` swallows `OperationCancelledException`s
- 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
Assessment
This issue has not been assessed yet.