Skip to content

SetAction overloads should have Task options without CancellationToken #2562

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

Open
ericstj opened this issue May 24, 2025 · 1 comment
Open
Labels
Area-API bug Something isn't working

Comments

@ericstj
Copy link
Member

ericstj commented May 24, 2025

Folks might accidentally pass an async lambda that doesn't accept a cancellation token and the compiler will treat it as async void, which will in turn be treated as a synchronous action by System.Commandline. Many places in our API we treat CancellationToken as optional, we should do so here. Please add overloads that accept Func returning Task without a cancellation token.

@jonsequitur jonsequitur added bug Something isn't working Area-API labels May 27, 2025
@adamsitnik
Copy link
Member

In the past, the CancellationToken was exposed via a method of InvocationContext: sample

rootCommand.SetHandler(async (context) =>
{
    string? urlOptionValue = context.ParseResult.GetValueForOption(urlOption);
    var token = context.GetCancellationToken();
    returnCode = await DoRootCommand(urlOptionValue, token);
});

And majority of code that I was updating in dotnet/* was not obtaining this token and passing it further.

The reason why I made it mandatory was that I wanted the compiler to produce a warning when it's not passed further (CA2016).

Folks might accidentally pass an async lambda that doesn't accept a cancellation token and the compiler will treat it as async void,

I was not aware of that, actually I wanted to ask you for a repro and then realized that following code compiles just fine:

Argument<string> address = new("address");
RootCommand command = new() { address };

command.SetAction(async paseResult =>
{
    IPAddress ipAddress = IPAddress.Parse(paseResult.GetValue(address)!);
    using (Ping pinger = new Ping())
    {
        PingReply reply = await pinger.SendPingAsync(ipAddress);
        Console.WriteLine(reply.Status);
    }
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-API bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants