Async.Await overload (esp. AwaitTask without throwing AggregateException)
I propose including AwaitTaskCorrect (maybe with another name)
open System
open System.Threading.Tasks
type Async with
static member AwaitTaskCorrect(task : Task) : Async<unit> =
Async.FromContinuations(fun (sc,ec,cc) ->
task.ContinueWith(fun (task:Task) ->
if task.IsFaulted then
let e = task.Exception
if e.InnerExceptions.Count = 1 then ec e.InnerExceptions.[0]
else ec e
elif task.IsCanceled then
ec(TaskCanceledException())
else
sc ())
|> ignore)
static member AwaitTaskCorrect(task : Task<'T>) : Async<'T> =
Async.FromContinuations(fun (sc,ec,cc) ->
task.ContinueWith(fun (task:Task<'T>) ->
if task.IsFaulted then
let e = task.Exception
if e.InnerExceptions.Count = 1 then ec e.InnerExceptions.[0]
else ec e
elif task.IsCanceled then
ec(TaskCanceledException())
else
sc task.Result)
|> ignore)
// examples
let mkFailingTask exn = Task.Factory.StartNew<_>(fun () -> raise exn)
let test1 taskAwaiter =
async {
try
return! taskAwaiter (mkFailingTask (ArgumentException()))
with
| :? ArgumentException -> return true
| :? AggregateException -> return false
} |> Async.RunSynchronously
test1 Async.AwaitTask // false
test1 Async.AwaitTaskCorrect // true
let test2 taskAwaiter =
async {
try
return! taskAwaiter (mkFailingTask (AggregateException("kaboom!")))
with
| :? AggregateException as e when e.Message = "kaboom!" -> return true
| :? AggregateException -> return false
} |> Async.RunSynchronously
test2 Async.AwaitTask // false
test2 Async.AwaitTaskCorrect // true
The existing way of approaching this problem in F# is copying the snippet in your project, ref : jet/equinox#198
Pros and Cons
The advantages of making this adjustment to F#:
- Provides a better implementation of Async.AwaitTask where the actual exception of a failing task is passed to the async mechanism.
- Works as C# would handle taks
The disadvantages of making this adjustment to F# are:
- Might be confusing having 2 AwaitTask
- If AwaitTask is "fixed" then is a breaking change
Extra information
Estimated cost (XS, S, M, L, XL, XXL): No idea
Related suggestions: at this moment I don't know of any other related suggestions might edit in the future
Affidavit (please submit!)
Please tick this by placing a cross in the box:
- [X] This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
- [X] I have searched both open and closed suggestions on this site and believe this is not a duplicate
- [X] This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it.
Please tick all that apply:
- [X] This is not a breaking change to the F# language design
- [X] I or my company would be willing to help implement and/or test this
If breaking AwaitTask it would be good to fix up another of it's oddities at the same time, that it can't be cancelled: https://github.com/dotnet/fsharp/pull/7357
This is related to https://github.com/dotnet/fsharp/pull/3257
Indeed, I discussed this quirk there as well.
I'm skeptical about changing the exception type here...
I'm also a bit skeptical about calling it Correct
Maybe we can add an Overload and a Method parameter (unwrapException)?
@matthid I’ll agree that the “Correct” part has to go, I was just honoring the Original Poster of the snippet.
The overload with a param that triggers the new behavior seems fine to me.
I checked that PR real quick and I’m not sure if it is exactly this, but really related should have been merged 😱
How do you deal with this? Is there any library that compensates this design right now?
How do you deal with this? Is there any library that compensates this design right now?
I usually have some extension-methods to check for inner exceptions and/or unwrap the exception if needed
I agree this should be solved. We need a better name - adding an optional argument seems reasonable though is a binary breaking change (though I believe it can be arranged that it's not a source breaking change on re-compilation).
I will mark this as approved in principle - would be great to have it as a community-led contribution
For the IsFaulted path should the code check that AggregateException is being thrown?
Related, and a potential alternative path (that is: a community led TaskEx lib): https://github.com/fsprojects/FSharp.Control.TaskSeq/issues/128.
We need a better name. Perhaps if we had an overloaded Async.Await(task) overloaded with
- Async.Await(Async<T>)
- Async.Await(Task<T>) -- adjusted Async.AwaitTaskCorrect semantics
- Async.Await(ValueTask<T>)
- Async.Await(task-like)
I'll adjust the name of this suggestion
Note that Async.Await(ValueTask) will be NS2.1 only.
Is it wise to put all these overloads in Async? It may be hard to discover, people would probably expect task-like operations to be in Task.xxx, and same for ValueTask.
We could add them symmetrically, similarly to how it is done for collection conversions
Async.Await(ValueTask) ValueTask.toAsync
I decided to add them in IcedTasks under AsyncEx if anyone wants to give it a try.
What about other Async functions doing surprising wrapping of aggregate exceptions?
For instance, I think we need something like:
type Async with
static member StartImmediateAsTaskCorrect (computation: Async<'T>, ?cancellationToken) : Task<'T> =
let cancellationToken = defaultArg cancellationToken Async.DefaultCancellationToken
let ts = TaskCompletionSource<'T> ()
Async.StartWithContinuations (
computation,
ts.SetResult,
(function
| :? AggregateException as agg -> ts.SetException agg.InnerExceptions
| exn -> ts.SetException exn),
(fun _ -> ts.SetCanceled ()),
cancellationToken)
ts.Task
Maybe I should create a separate issue.
Think that's best covered under a separate issue (and revisiting Async.DefaultCancellationToken stuff can be talked about there too...)