Skip to content

[Analyzer] Flag catch blocks for TaskCanceledException without OperationCanceledException #83406

@stephentoub

Description

@stephentoub

With 20/20 hindsight, TaskCanceledException was a mistake. It derives from OperationCanceledException, and the intent was that it's just an OperationCanceledException that carries a little more information folks might want to see, namely the Task that was canceled and triggered the exception. However, it's very common for the boundary between an OCE and a TCE to be very fluid, where the same operation might result in one or the other depending on when cancellation occurred (e.g. an up-front cancellationToken.IsCancellationRequested check vs an await on a Task manually completed with SetCanceled on its completion source), and that fluidity not only results in race conditions where sometimes you get one exception and sometimes you get the other, changes to the code over time often result in happening to throw one vs the other.

The net result of this is that code trying to catch and handle cancellation exception should always do so for OperationCanceledException. If code does want to do something special for TaskCanceledException, it can do so in addition but shouldn't do instead of.

We should add an analyzer that flags catch (TaskCanceledException) if there's no corresponding catch (OperationCanceledException) that would back stop it.

// Ok
try { ... }
catch (OperationCanceledException) { ... }

// Ok
try { ... }
catch (OperationCanceledException oce)
{
    if (oce is TaskCanceledException tce) { .. }
}

// Ok
try { ... }
catch (TaskCanceledException) { ... }
catch (OperationCanceledException) { ... }

// Diagnostic
try { ... }
catch (TaskCanceledException) { ... }

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions