I don't think this question is a duplicate of "Proper way to deal with exceptions in DisposeAsync".
Let's say my class that implements IAsynsDisposable
because it has a long-running background task, and DisposeAsync
terminates that task. A familiar pattern might be the Completion
property, e.g. ChannelReader<T>.Completion
(despite ChannelReader
doesn't implement IAsynsDisposable
).
Is it considered a good practice to propagate the Completion
task's exceptions outside DisposeAsync
?
Here is a complete example that can be copied/pasted into a dotnet new console
project. Note await this.Completion
inside DisposeAsync
:
try
{
await using var service = new BackgroundService(TimeSpan.FromSeconds(2));
await Task.Delay(TimeSpan.FromSeconds(3));
}
catch (Exception ex)
{
Console.WriteLine(ex);
Console.ReadLine();
}
class BackgroundService: IAsyncDisposable
{
public Task Completion { get; }
private CancellationTokenSource _diposalCts = new();
public BackgroundService(TimeSpan timeSpan)
{
this.Completion = Run(timeSpan);
}
public async ValueTask DisposeAsync()
{
_diposalCts.Cancel();
try
{
await this.Completion;
}
finally
{
_diposalCts.Dispose();
}
}
private async Task Run(TimeSpan timeSpan)
{
try
{
await Task.Delay(timeSpan, _diposalCts.Token);
throw new InvalidOperationException("Boo!");
}
catch (OperationCanceledException)
{
}
}
}
Alternatively, I can observe service.Completion
explicitly in the client code (and ignore its exceptions inside DiposeAsync
to avoid them being potentially thrown twice), like below:
try
{
await using var service = new BackgroundService(TimeSpan.FromSeconds(2));
await Task.Delay(TimeSpan.FromSeconds(3));
await service.Completion;
}
catch (Exception ex)
{
Console.WriteLine(ex);
Console.ReadLine();
}
class BackgroundService: IAsyncDisposable
{
public Task Completion { get; }
private CancellationTokenSource _diposalCts = new();
public BackgroundService(TimeSpan timeSpan)
{
this.Completion = Run(timeSpan);
}
public async ValueTask DisposeAsync()
{
_diposalCts.Cancel();
try
{
await this.Completion;
}
catch
{
// the client should observe this.Completion
}
finally
{
_diposalCts.Dispose();
}
}
private async Task Run(TimeSpan timeSpan)
{
try
{
await Task.Delay(timeSpan, _diposalCts.Token);
throw new InvalidOperationException("Boo!");
}
catch (OperationCanceledException)
{
}
}
}
Is there a concensus about which option is better?
service.Completion
, then maybe I should not even implementIAsyncDisposable
and let the client track the completion status and any errors? Tks for the discussion! :)Completion
. But the main drawback is that you can't enforce the consumer of your class to callawait service.Completion;
, which can lead to swallowing exception. The consumer might think that the operation has succeeded even though it's not.