dotnet / dotnet/command-line-api

All CLIs should support cancellation

Open
#2,155 7 comments 1 reaction 0 assignees View on GitHub
needs discussion
Dominant language
C#
Stars
3.7k
Forks
428
PR merge metrics
No merged PRs in 30d

Description

Either always or by default CLIs should support cancellation with Ctl-C. It is an extremely rare side case that is not the right thing. I do not suggest this will be an easy space to resolve, but at least async over sync is lighter than it used to be. I struggle to come up with any use case where Ctl-C should be ignored in a CLI. Also, CliAuthors should find a pit of success around cancellable CliActions.

There are also only very limited cases where the CLI author needs control returned to them as a task. The CLI author will almost always just await the task before they can do other work.

Also, it will be rare that `CliAction`s need to be async for reasons other than this is the way we traditionally did cancellation.

Also, our current API around this is confusing. `CliAction` is an abstract class with `Invoke` and `InvokeAsync` methods. Which one is called depends on whether `Invoke` or `InvokeAsync` is called on the root command. Any single `CliAction` is logically either sync of async. If the user then calls the other on the root command, their operation will not run. This may have been necessary in our previous design, but I believe we can fix it in the new CliAction design and our partnership with Runtime Libraries.

As a strawman (I know this is not completely correct, it is to spur a conversation):

* Have a single invoke method on CliAction (see below)
* Have only an `Invoke` method on CliConfiguration or RootCommand
* The `CliConfiguration.Invoke` method always calls `InvocationPipeline.InvokeAsync` and always supports cancellation. The actual code depends on the CliAction decision below.
* Pre and post actions are also cancellable, and any redirection/conditional execution (see #2154) follows the same code path (other than execution)
* Actual async handling of input is considered a niche case and not included in V1 (scenarios requested)
* What scenarios are there where you want a task from `CliConfiguration.Invoke`/`RooCommand.Invoke` if cancellation is handled for you.

CliAction:

* If a single `Invoke(ParseResult parseResult, CancellationToken cancellationToken)` action on `CliAction`
* If the user has fast operation and nothing within takes a cancellation token, they can discard the cancellationToken
* If the user has several potentially slow steps (or a loop) they can periodically check the cancellation token
* If the user has an operation that can do some code that supports cancellation they can pass it and check on return if there are more operations in their `CliAction` (I am a bit fuzzy on this and would turn to experts)
* We need a pattern for setting the return code

OR

* If a single `InvokeAsync(ParseResult parseResult, CancellationToken cancellationToken)` action on `CliAction`
* This is more complicated for users in the common case

I do not know what this should actually look like, but the CliAuthor should be able to create cancellable tasks as easily as not cancellable and there should be no requirement outside the specific action needed to support cancellation for a single specific `CliAction`

This is really important for ecosystem consistency.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.