Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal]: Then or ContinueWithResult extension method for Task #58692

Closed
gulshan opened this issue Sep 5, 2021 · 13 comments
Closed

[API Proposal]: Then or ContinueWithResult extension method for Task #58692

gulshan opened this issue Sep 5, 2021 · 13 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Threading.Tasks needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Milestone

Comments

@gulshan
Copy link

gulshan commented Sep 5, 2021

Background and motivation

Task currently has a method group named ContinueWith, which supports doing something when the task is finished and result is available or there is an exception. But it takes the main Task as a parameter and successful completion of the Task has to be checked and Result has to be accessed manually.

The proposal is to give a new extension method for Task and Task<T>, which will called with the result of the Task, only if the task has been completed successfully. The name of the method can be Then or ContinueWithResult. It will return a task combining the given task and the continuation. If the continuation is also a task, this will merge them into a new task and return.

With Then, the method chaining will become easier, which is a bit difficult with await and quite cumbersome with current ContinueWith method.

API Proposal

namespace System.Threading.Tasks
{
     public static class TaskExtensions
     {
          public static Task Then(this Task task, Action continuationAction);
          public static Task Then<TResult> (this Task<TResult> task, Action<TResult> continuationAction);
          public static Task<TNewResult> Then<TResult,TNewResult> (this Task<TResult> task, Func<TResult,TNewResult> continuationFunction);
     }
}

API Usage

// Read all text from file asynchronously
File.ReadAllTextAsync(filepath)
    .Then(text => {
        // Use the file text
        Console.WriteLine(text);
        return text.Length;
    });

Risks

No response

@gulshan gulshan added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Sep 5, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Sep 5, 2021
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@omariom
Copy link
Contributor

omariom commented Sep 5, 2021

There is already such method .

@Frassle
Copy link
Contributor

Frassle commented Sep 5, 2021

@omariom that is not the method that this issue describes.

I nearly always end up writing an extension method to do this, which cause I'm an FP nerd I always call bind because this is monadic bind. It's useful, gives the same semantics as await pretty much but in a method calling/function passing style rather than a sequential code/keyword style.

@stephentoub
Copy link
Member

stephentoub commented Sep 5, 2021

which is a bit difficult with await

Why?

Your example:

// Read all text from file asynchronously
File.ReadAllTextAsync(filepath)
    .Then(text => {
        // Use the file text
        Console.WriteLine(text);
        return text.Length;
    });

becomes:

string text = await File.ReadAllTextAsync(filepath);
Console.WriteLine(text);
return text.Length;

Such Then methods can also trivially be implemented with await:
https://devblogs.microsoft.com/pfxteam/implementing-then-with-await/
if you really want these semantics and don't want to use await for some reason.

@danmoseley
Copy link
Member

@gulshan this seems resolved, do you want to close?

@gulshan
Copy link
Author

gulshan commented Sep 10, 2021

I would still like to have this extension method available in standard library, if not now, maybe sometime in future. But it is your decision to make at the end.

@Jozkee Jozkee added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed untriaged New issue has not been triaged by the area owner labels Sep 10, 2021
@Jozkee Jozkee added this to the Future milestone Sep 10, 2021
@theodorzoulias
Copy link
Contributor

Such Then methods can also trivially be implemented with await:
https://devblogs.microsoft.com/pfxteam/implementing-then-with-await/
if you really want these semantics and don't want to use await for some reason.

@stephentoub solving this problem with async/await is not 100% satisfying, because it results to all but the first exceptions stored inside the task to get swallowed. Having to attach continuations to Task.WhenAll/Parallel.ForEachAsync/IDataflowBlock.Completion tasks that may contain more than one exceptions is not totally rare, and the proposed Then method could make the experience of writing these continuations less painful.

@stephentoub
Copy link
Member

because it results to all but the first exceptions stored inside the task to get swallowed

That is the nature of async/await. And even if these helpers flowed multiple exceptions through to the returned Task, the 99.9% case of consuming the resulting Task will itself be with await, at which point the effort to flow the additional exceptions is for naught. I understand your point, but I don't personally see this as sufficient motivation; wanting such Then methods instead of await is already niche, and then wanting them because of a desire to flow multiple exceptions is a niche corner of niche ;-)

@theodorzoulias
Copy link
Contributor

@stephentoub this is my best attempt to implement a Then method that propagates all the errors. Could you take a look and see if it has the correct behavior? By correct I mean the behavior that this method would have if it was available in the standard library.

public static Task<TNewResult> Then<TResult, TNewResult>(
    this Task<TResult> source,
    Func<TResult, TNewResult> continuationFunction,
    CancellationToken cancellationToken = default,
    TaskContinuationOptions continuationOptions = TaskContinuationOptions.None,
    TaskScheduler scheduler = null)
{
    scheduler ??= TaskScheduler.Current;
    return source.ContinueWith(t =>
    {
        if (t.IsCanceled) t.GetAwaiter().GetResult(); // Propagate the correct token
        if (t.IsFaulted)
        {
            var tcs = new TaskCompletionSource<TNewResult>();
            tcs.SetException(t.Exception.InnerExceptions);
            return tcs.Task;
        }
        var newResult = continuationFunction(t.Result);
        return Task.FromResult(newResult);
    }, cancellationToken, continuationOptions, scheduler).Unwrap();
}

@stephentoub
Copy link
Member

I think that's more complicated than it needs to be:

source.ContinueWith(t => t.IsCompletedSuccessfully ? Task.FromResult(continuationFunction(t.Result)) : t,
    cancellationToken, continuationOptions, scheduler ?? TaskScheduler.Current).Unwrap();

@gulshan gulshan changed the title [API Proposal]: Then or ContinueWithResult extension method for result [API Proposal]: Then or ContinueWithResult extension method for Task Oct 4, 2021
@theodorzoulias
Copy link
Contributor

This gives this error on the ternary operator:

Error CS0173 Type of conditional expression cannot be determined because there is no implicit conversion between 'System.Threading.Tasks.Task<TNewResult>' and 'System.Threading.Tasks.Task<TResult>'.

It's a pain when the continuation has different result type than the source task.

@stephentoub
Copy link
Member

Ah, I missed that you were trying to change the result type. Then what you have looks like a reasonable implementation to maintain multiple exceptions for whatever library you're using this in.

@stephentoub
Copy link
Member

I'm going to close this. While I appreciate the desire for such methods and your submitting a proposal, async/await is the recommended approach, if you really want methods there exists ContinueWith, and the Then semantics can be trivially layered on top of one of those. We don't want yet another approach built-in. Thanks.

@stephentoub stephentoub closed this as not planned Won't fix, can't repro, duplicate, stale Sep 2, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Oct 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Threading.Tasks needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Projects
No open projects
Development

No branches or pull requests

8 participants