Repository navigation
Remove AggregateException wrapping in Async CE for extension and prevent threadpool transition #129
Description
Activity
Fringe ramblings which you may consider related: I tend to have an
Async.startImmediateAsTaskhelper that I try to route the starting of child Tasks through withinasync {s :-module private Async = let startImmediateAsTask ct computation = Async.StartImmediateAsTask(computation, cancellationToken = ct)Aside: the term
Immediatenever really spoke to me; I tended to quickly discount it from consideration. I assume that naming is probably intentional. While I doubt aliasing an established name is likely to pass any scrutiny, perhaps naming such a helper something likeAsync.executeTaskmight express the intention better? (I'd call itAsync.runAsTask, but for me the use ofRunconnotesTask.Run, i.e. implies an invocation via the Thread Pool)Related: Potentially would advocate for inclusion of any
startImmediateAsTaskhelper in a shims lib alongside #128Yes, before I wrote this extension, I wasn't aware of the issues with
Async.AwaitTask. This can clearly be improved. I'll probably need a test that showcases how this goes off the rails, to ensure the implementation ofAwaitTaskCorrect(by whatever other name) is, well, correct.As with #128, we should consider placing these in a separate library. Though for the sake of this one being a no-dependency library by choice, for now we'll just implement it here (and/or copy or link it from whatever other library we conceive).
- changed the title
[-]'Fix' Async `for` semantics[/-][+]Remove AggregateException wrapping in Async CE `for` extension and prevent threadpool transition[/+]on Dec 14, 2022 This can clearly be improved. I'll probably need a test that showcases how this goes off the rails, to ensure the implementation of AwaitTaskCorrect (by whatever other name) is, well, correct.
Yes, the honoring of Async.CancellationToken aspect to terminate the wait is notably subtle to me and makes covering it well worthwhile.
But by the time you've done that, the canonical impl will likely see some change (and I bet the perf can be improved)Though for the sake of this one being a no-dependency library by choice
I'm in a similar quandary with Equinox - the core lib is tiny and making it depend on another lib only for AwaitTaskCorrect feels like jumping the shark and going leftpadesque. There are 5 other libs that need both TaskSeq and AwaitTaskCorrect. But the bottom line is that having a separate lib, no matter how small, is simply the right thing to do. UNLESS/until it goes into FSharp.Core.... (see conclusion below)
FsKafka is another case where the lib is a 400 line slab that has a stupid other file that dilutes that simplicity and directness thanks to AwaitTaskCorrect
FSharp.AWS.DynamoDB has a copy too, and I really would like for the world to have a way of knowing that a given exe has only one set of AwaitTask semantics in play when reasoning about stack traces and the inevitably hairy cancellation and/or hang issues (see dotnet/fsharp#13165 - a large part of whittling that down was doing stupid busywork diffing to rule that sort of stuff out). (for that lib, the larger issue is that it should probably present a
Tasklayer, and then layer its currentAsyncAPI over that - atm Equinox is having to do a redundant layer of Task over Async which then immediately drops to Task in the underlying impl within FSAWSDDB)Having said all that, I respect the value of not having a package dependency and will likely distribute a copy of the final AwaitTaskCorrect into FSharo.AWS.DynamoDB, FsKafka, Equinox and Propulsion if that's what TaskSeq does (and will argue against making TaskSeq's version public to avoid people havign to self-discipline to not take a depedency on TaskSeq only for AwaitTaskCorrect). But I think I am saying I'm voting for
*.ignoreandAsync.toTaskto go in a single nuget until they can go into FSharp.Core where they should really be for everyone's sake.Related: Potentially would advocate for inclusion of any
startImmediateAsTaskhelper in a shims lib alongside #128
Addendum to this re fsprojects/FSharp.Control.AsyncSeq#74 (comment)- An (Async|Task).parallelThrottled would go well with
Async.runAsTaskand/or(Async|Task|ValueTask).ignore(forcing specification of a cancellationtoken and a degree of parallelism to be considered when going parallel). At present I have some ugly cases where I am abusing Async.Parallel within code that's Task-based. It might be a stretch for them to go into FSharp.Core for reasons of redundancy as per Don's comments, but having a clear surface area that makes one consider cancellation in all cases, and degreeOfParallelism in cases of going parallel would help in general IMO - are
iterParallel(Async)?andmapParallel(Async)?missing from the roadmap? - could consider the notion of a generic
Async.throttleas a way to constrain parallelism vs having a degreeOfParallism aspect, though a) that's likely less optimal and b) arguably that is less likely to have users end up in good designs - as it stands I will cart likely cart around a
mapAsyncParallelThrottledhelper as per Parallel sequence runs consequentially (non-parallel) FSharp.Control.AsyncSeq#74 (comment) until such time as either that becomes directly available, or there's amapAsyncParallelthat I then constrain via having an Async.Throttle and/or Task-awaitable lightweight semaphore wrapper - wrt all these helpers, having
AwaitTaskCorrect/Async.toTaskin FSharp.Core (perhaps as part of the MS 7.0 push? https://twitter.com/rbartelink/status/1597511316176252928) would allow these smaller helpers/API surface area considerations to be considered more clearly than having the need for that to be resolved be lurking in the background of all these fringe issues.
- An (Async|Task).parallelThrottled would go well with
Thanks for all the input, some work is cut out for me :).
But I think I am saying I'm voting for *.ignore and Async.toTask to go in a single nuget until they can go into FSharp.Core where they should really be for everyone's sake.
Alternatively: we can take a source dependency instead. That way it can be internal.
are iterParallel(Async)? and mapParallel(Async)? missing from the roadmap?
All of AsyncSeq is potentially on the roadmap, but some design decisions there I don’t want to repeat. My first goal is to complete the ‘low hanging fruit’ roadmap, based on
Seq‘s surface area.This gives us
task-like behaviour. Like F#task, it doesn’t support throttling or parallelism, but unliketaskthere’s more of a use case to support it explicitly. However, tasks are hot started, which makes this a different kind of challenge (since TaskSeq has both task-like and seq-like behaviour, it’s a blend between the two essentially).See also #77 for the differences.
Ah, I missed the mention of the fact that parallelism is off the table for now in https://github.com/fsprojects/FSharp.Control.TaskSeq#further-reading-iasyncenumerable - focusing on getting stuff mapped out in the core feature set absolutely makes sense, especially given the non-trivial nature of the ancillary aspects as highlighted by this very issue.
Reacted by Abel BraaksmaJust FYI: the
StartAsTaskandStartImmediateAsTaskpoints that you brought up here escaped my attention somehow. They're now separately under the umbrella of #135.This thread can then solely focus on the issues around
AggregateExceptionandAwaitTaskCorrect(needs a better name).👍 Apologies for mixing the concerns in the first instance. Putting this here as this thread has already run amok; not sure if it's really
TaskSeqbusiness but it would be good to see it result in either an agreed set of helpers, or a summary document somewhere. Some of this is covered in #128, but I'd like to explore the surface area a tiny bit more here first before this reverts to being a simple issue about theforimpl being corrected...TL;DR there needs to be a canonical set of helpers somewhere that emphasize:
- not being painful to use with piping
- with good names
TaskSeqcurrently
a) uses/should use the bulk of these
b) exposes some of the othersBelow the fold, I have itemised the APIs I believe should be considered for wrapping.
My hope is that we can:
a) converge on what APIs go into theTaskExsignature set so it can go into a fresh issue
b) Close #128 as it is currently confusing about whether it is trying to addressignore,awaitTask, or the whole lib, pointing to ☝️
c) Close this in favour of a fresh issue that links toAwaitTaskCorrectand #135
ignoreAsync.Ignorehas always been ugly and undiscoverable. While I tend to dolet! _ = <async stuff I want to ignore result of>, it's commonly the last expression in a function, and having to dolet! _ = <thing I'm wrapping> in ()is too much.=> As appears to the be direction taken in this PR, any given construct should have a correct
ignoreimpl, that corrects observes exceptions etcmodule Async = let inline ignore (a : Async<'t>) = Async.Ignore module Task = let inline ignore (t : Task<'t>) = ... module ValueTask = let inline ignore (t : ValueTask<'t>) = ...StartAsTask/StartStartnormally doesn't confuse people - It's clear it's in the thread pool, and cancellation can normally be swept under the rug.StartAsTaskhas signaturecomputation * ?taskCreationOptions *. ?cancellationToken- passing a cancellation token requires
Async.StartAsTask (computation, ?cancellationToken = ct) - the name does not scream thread hop/thread pool
- passing a cancellation token should not be glossed over, ever;
Asyncbeing replaced withTaskeverywhere does not change the importance of treating the correct propagation of cancellation token as a first class concern that should always be on the table
- passing a cancellation token requires
Suggested API:
module Async = let inline runAsTask ct (a : Async<'t>) = Async.StartAsTask(a, cancellationToken = ct)StartImmediateAsTaskAs noted, this should be the default way in which
Asyncs are started.- Because it is not starting a thread, it does not have a
taskCreationOptionsarg so it happens one can use it directly for piping. - The
immmediatebit does not align with anything I'm aware of, although that also means it's meaning, once understood, is not ambiguous. I'd be suggestingexecuteas a verb to imply "do it here and now" alongsiderunas a better name igf the precedent had not been established.
My best suggestion is thus to have:
module Async = let inline startImmediateAsTask ct (a : Async<'t>) = Async.StartImmediateAsTask(a, cancellationToken = ct) // ALTERNATELY, esp if runAsTask name survives: let inline executeAsTask ct (a : Async<'t>) = Async.StartImmediateAsTask(a, cancellationToken = ct)AwaitTaskThe default impl is baked and wrong, but adding an overload is both desirable and questionable
- this lib has
Async.toTaskandTask.ofAsync
module Async = let inline ofTask (t : Task<'t>) : Async<'t> = AwaitTaskCorrect t module Task = let inline toAsync (t : Task<'t>) : Async<'t> = Async.ofTask tAsync.Parallel
The degree of parallelism parameter was added late in the game, but is critical - adding an arbitrary number of items to the threadpool should be a very questionable act.
- pipelining is painful
- having Throttled in the name is pretty well established https://www.compositional-it.com/news-blog/improved-asynchronous-support-in-f-4-7
- before v FSharp.Core v 6.0.6, [there was a stack overflow bug that can tear down the process](// Async.Parallel stack overflow on cancellation of ~2000 uncompleted computations dotnet/fsharp#13165) if >1200 items are started with a throttle and cancellation is triggered quickly
module Async = let parallelThrottled dop computations = Async.Parallel(computations, maxDegreeOfParallelism = dop)Further puzzlers wrt
Parallel(which probably mean this should be excluded from the surface area of any set of helpers):- A common case is to use this to run but await failure of multiple
Async<unit>tasks, having to do anAsync.Ignore<unit[]>is ugly for that - How do you swap back/forth from that to
Task, considering cancellation tokens and unwrapping AggregateException
Maybe you can copy that whole text as a new issue? It feels like it actually belongs in #128, no? Basically the whole idea of agreeing on a surface area for Task/Async, likely in its own repo and package.
Maybe I should just go set one up and we can continue there?
Forgot I own #128 🤡, that does make kinda sense. My intent was to transplant all this into a fresh issue.
But I did want for us to have a brief pre-discussion about the outline surface area before spreading it widely (and re-atting Don and TheAngryByrd etc)
I appreciate you're probably trying to avoid jumping straight back into this after expending lots of time and effort only yesterday, but making sure that we've both happy with the first cut before I create it afresh would be ideal, even if that means holding off for some days (it might also make sense to defer this until we (aka you) have validated that an impl of
TaskSequsingAwaitTaskCorrect+StartImmediateAsTaskinternally actually works correctly; it might also yield some minor additional insights as to what the surface area needs to be)....- added a commit that references this issue
on Mar 9, 2026 🤖 This is an automated response from Repo Assist.
After reviewing the current state of
AsyncExtensions.fs, I've submitted a fix for theAggregateExceptionwrapping issue described here.What was wrong
AsyncExtensions.fsusedAsync.AwaitTaskto await the task produced byTaskSeq.iterAsync.Async.AwaitTaskunconditionally wraps all task faults inAggregateException, which meant thattry/catchblocks inasync {}expressions iterating over ataskSeqwithforcould not catch the original exception type:async { for item in myTaskSeq do try doSomething item with :? InvalidOperationException -> () // this catch was never hit — got AggregateException instead }
The fix
I replaced
Async.AwaitTaskwith a privateawaitTaskCorrecthelper usingAsync.FromContinuationsthat:- Unwraps single inner exceptions from
AggregateExceptionbefore routing to the error continuation - Passes through multi-inner-exception
AggregateExceptionas-is - Properly routes task cancellation to the async cancellation continuation
This is the standard
AwaitTaskCorrectpattern described in fslang-suggestions#840.Two regression tests have been added to verify the fix.
The PR is at branch
repo-assist/fix-async-awaitTask-129-2026-03.Note:
Async.StartImmediateAsTask(the fix from #135) was already in place in the current code — that part of the issue was already resolved.Generated by Repo Assist · ◷
To install this agentic workflow, run
gh aw add githubnext/agentics/workflows/repo-assist.md@346204513ecfa08b81566450d7d599556807389f- Unwraps single inner exceptions from
@dsyme For avoidance of doubt, IMO the PRs being stacked here wrt the 'TaskEx' set as described in #139 are questionable - generic small task helpers in this lib that are orthogonal in nature to the root purpose of this library, i.e. having AwaitTaskCorrect in here is not a full answer for the overall ecosystem - that needs to either go in a separate lib, or perhaps in FSharp.Core (or we have conventional names for private/inline helpers per other infra library).
My specific concern about merging this one is that there are also other
AwaitTasks besides the one it found around here, and those should be consistent for sanity too.I'm considering getting around to doing the
AwaitTaskCorrectinFSharp.Core, which is the real answer here, but it certainly won't be this week.
At present the current impl of
forwithinasync {expressions raises two concerns for me:AwaitTaskCorrectsemantics would be preferable to promulgating usage ofAsync.AwaitTaskin a place that most people will not necessarily even infer that it's in play.Async.StartAsTaskincludes an unnecessary transition to the Thread Pool (with a potential context switch?) which could instead safely be replaced withAsync.StartImmediateAsTaskin this context. EDIT: moved to separate thread #135