Skip to content

feat(Async): Add exception-unwrapping Await#19785

Open
bartelink wants to merge 5 commits into
dotnet:mainfrom
bartelink:async-await
Open

feat(Async): Add exception-unwrapping Await#19785
bartelink wants to merge 5 commits into
dotnet:mainfrom
bartelink:async-await

Conversation

@bartelink
Copy link
Copy Markdown

@bartelink bartelink commented May 21, 2026

Implements Async.Await for Task, Task<'T>, ValueTask and ValueTask<'T> (aka the community AwaitTaskCorrect implemention)

Key differentiation from Async.AwaitTask is that AggregateExceptions are unwrapped such that a try ... with <ExceptionType> -> will type-match correctly.

Key distinction from the canonical implementation (which derives from https://www.fssnip.net/7Rc/title/AsyncAwaitTaskCorrect) is that the implementation is intended to have 1:1 matching of all stacktrace preservation properties borne by AwaitTask (and continue to track that over time).

NOTE one key implementation decision is that this PR does NOT attempt to resolve #2127 so:

  1. if the computation workflow's CT is cancelled at the point where Await is invoked, normal cancellation semantics as per AwaitTask apply:
    • exception continuation is passed an OperationCanceledException
    • the Task in question's Result will go unobserved
  2. if a cancellation of the computation workflow via it's ambient cancellation token is triggered during the course of the Await, it will (like AwaitTask):
    • NOT abort and abandon the observation Task
    • instead, it will wait [as a C# await would] until such time as the Task completes (either successfully, with a fault, or via cancellation)

Checklist

  • Test cases added
  • Release notes entry updated
  • Add documentation cross-links and strongly-implied deprecation flagging conveying that:
    • AwaitTask will always yield an AggregateException
    • Await should be the used in preference for new code
    • Await can technically still propagate an AggregateException

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

❗ Release notes required


✅ Found changes and release notes in following paths:

Change path Release notes path Description
src/FSharp.Core docs/release-notes/.FSharp.Core/11.0.100.md

@bartelink bartelink force-pushed the async-await branch 7 times, most recently from b2b2b6a to b710cc1 Compare May 21, 2026 14:58
@bartelink
Copy link
Copy Markdown
Author

bartelink commented May 21, 2026

@T-Gro if you and/or others can give this a quick scan please, I'd like to confirm:

  1. nobody else has work in flight (yes, a bit late for that!)
  2. the rough approach is viable
  3. the key impl decision is reasonable:
    • no attempt to get too clever and have the API react to Async cancellation by abandoning waits, i.e. if you have code that does not propagate Async.CancellationToken to a Task start, Await will hang just as AwaitTask
      • BUT this is OK as abandoning in-flight tasks and/or the controlled disposal of associated resources/compute would not be strictly better
      • It aligns with e.g. how Async.Parallel waits for correct teardown/completion of all in-flight executions before yielding a result and/or completing the honoring of cancellation
    • Best practice recommendation will instead be to use Async.StartTaskImmediate, which will
  4. a rough indication of whether I should leave the xmldoc as is, or attempt to complete the rough tasks I've laid out in the checklist in the OP

TL;DR the overall proposition

  1. Await is just AwaitTask with unwrapping, zero other semantic change. => A better default to use where you'd otherwise use AwaitTask
  2. Usage of AwaitTask and Await are both smells - can you be sure all Tasks that have been started were correctly wired into the computation tree's CT?
  3. In general, usage of Async.Await should be replaced with/migrated to Async.StartTaskImmediate, which surfaces the problem
  4. if there was an analyzer flagging Async.Await/AwaitTask -> Async.StartTaskImmediate migration opportunities, a closely related one would be flagging cases of task { flows that use let! and/or do! bindings against Async<'T>) rather than using Task.startAsyncImmediate (which forces passing a CT to Async.StartImmediateAsTask

CC @TheAngryByrd @gusty @njlr who have provided useful feedback/review on stuff in this space in recent times

@bartelink bartelink marked this pull request as ready for review May 22, 2026 13:37
Copilot AI review requested due to automatic review settings May 22, 2026 13:37
@bartelink bartelink requested a review from a team as a code owner May 22, 2026 13:37
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new Async.Await API to FSharp.Core for Task/Task<'T> (and ValueTask/ValueTask<'T> on netstandard2.1) that unwraps “egregious” single-inner AggregateExceptions so exception matching works as expected, while aiming to preserve existing AwaitTask stacktrace behavior.

Changes:

  • Implement Async.Await in async.fs by sharing the existing AwaitTask continuation machinery and selectively unwrapping single-inner AggregateExceptions.
  • Expand unit tests to exercise both AwaitTask and Await behavior (including AggregateException cases) and add ValueTask coverage under #if NETSTANDARD2_1.
  • Update FSharp.Core surface area baseline (partially) and add a release-notes entry.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/FSharp.Core.UnitTests/FSharp.Core/Microsoft.FSharp.Control/AsyncType.fs Converts many tests to run against both AwaitTask and new Await; adds new behavior-focused tests for AggregateException unwrapping and ValueTask.
tests/FSharp.Core.UnitTests/FSharp.Core.SurfaceArea.netstandard21.debug.bsl Adds Async.Await entries for netstandard2.1 Debug surface area baseline.
src/FSharp.Core/async.fsi Updates AwaitTask XML docs and adds new Async.Await API docs (including ValueTask overloads under netstandard2.1).
src/FSharp.Core/async.fs Implements Async.Await and refactors task-await internals to optionally unwrap single-inner AggregateExceptions.
docs/release-notes/.FSharp.Core/11.0.100.md Adds release note entry for new Async.Await.

Comment thread src/FSharp.Core/async.fsi
Comment thread src/FSharp.Core/async.fs Outdated
@bartelink bartelink force-pushed the async-await branch 2 times, most recently from 7aec92d to 9ab202c Compare May 22, 2026 14:03
@github-actions github-actions Bot added the AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files label May 22, 2026
Comment thread src/FSharp.Core/async.fs Outdated
Comment thread src/FSharp.Core/async.fs
if task.IsCompletedSuccessfully then
CreateReturnAsync(task.GetAwaiter().GetResult())
else
Async.Await(task.AsTask())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just AwaitTask ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming you mean delegate to AwaitTask true (and do same for the untyped), yes will do

@majocha
Copy link
Copy Markdown
Contributor

majocha commented May 22, 2026

I'm thinking about naming. Async.Await name does not suggest that it applies to Tasks only. Since it is already 4 times overloaded, maybe it makes sense to add an overload working with any awaitable, to have parity with C#? This can be discussed and done separately from this PR, of course. (Also SRTP can be a can of worms).

@bartelink
Copy link
Copy Markdown
Author

maybe it makes sense to add an overload working with any awaitable, to have parity with C#? This can be discussed and done separately from this PR, of course. (Also SRTP can be a can of worms).

@majocha Yes, this was already part of the brief as per the comment from @dsyme.

I'd personally need to research what it would entail, so if you or anyone wants to contribute an impl, feel free to hang a PR off this one.

But bottom line I agree it would be good for the support for tasklike things to be done either as part of this PR or as a very fast follow so the world only needs to validate the overloading works out cleanly once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

Async.AwaitTask does not cancel on workflow cancellation

3 participants