spectre.console icon indicating copy to clipboard operation
spectre.console copied to clipboard

CLI Command RunAsync support passing in/propagating a CancelationToken

Open Simonl9l opened this issue 3 years ago • 13 comments

Is your feature request related to a problem? Please describe. By convention in DotNet Core one wild supply a CancellationToken into an Async method when awaited!

Describe the solution you'd like Support passing in a CancelationToken such that is propagated to the AsyncCommand.ExecuteAsync and for that matter provide a ValidateAsync that also takes the token.

In this way a command handler can detect it's being canceled (via async Main for example)

Describe alternatives you've considered

One can make the organ available via static and class scoped but its not very well encapsulated.

Additional context N/A


Please upvote :+1: this issue if you are interested in it.

Simonl9l avatar Jan 24 '22 03:01 Simonl9l

Why? Can't you do this:

CancellationToken token; // From Somewhere else

AnsiConsole.Progress().StartAsync(async ctx =>
    {
          doStuff(token);
    });

kuhnboy avatar Jan 24 '22 19:01 kuhnboy

@kuhnboy, I think what @Simonl9l asked for here is:

  • ICommandApp.RunAsync(IEnumerable<string> args)
    • ➡️ ICommandApp.RunAsync(IEnumerable<string> args, CancellationToken cancellationToken = default(CancellationToken))
  • AsyncCommand.ExecuteAsync(CommandContext context)
    • ➡️ AsyncCommand.ExecuteAsync(CommandContext context, CancellationToken cancellationToken = default(CancellationToken))
  • AsyncCommand.ExecuteAsync<TSettings>(CommandContext context, TSettings settings)
    • ➡️ AsyncCommand.ExecuteAsync<TSettings>(CommandContext context, TSettings settings, CancellationToken cancellationToken = default(CancellationToken))
  • new:
    • AsyncCommand.ValidateAsync<TSettings>(CommandContext context, TSettings settings, CancellationToken cancellationToken = default(CancellationToken))

nils-a avatar Jan 24 '22 22:01 nils-a

Hi @kuhnboy & @nils-a thanks for the replies and clarifications...I've been away apologies for the wait in my reply!

in the version of the Spector.Console library I'm using 0.43.0 there seem to be no versions of these methods that support the CancellationToken argument.

Whist I "could" DI the Token in to the AsyncCommand constructor this seems very unconventional.

Are there plans to support passing through the CancellationToken as indicated in the prior reply?

Thanks

Simonl9l avatar Jan 31 '22 19:01 Simonl9l

@Simonl9l while there weren't any plans to implement CancellationTokens before this issue as far as I know, I personally see no reason not have them added in the future.

nils-a avatar Jan 31 '22 19:01 nils-a

@nils-a are you able to set any expectation of plans on what "future" might mean?

The underlying value proposition is that Id want to hook the CLI into PosixSignalRegistration per here such that I can cancel a command execution based on a SIGTERM/INT etc..

TBH Im still getting the hang of the library and started using become for the Console Tables etc.

As an aside are the also any plans to support Tab/Command completion in the CLI?

Simonl9l avatar Jan 31 '22 19:01 Simonl9l

are you able to set any expectation of plans on what "future" might mean?

Sadly, no. I find the idea intriguing, so I think I will look into it. Not sure when I have the cycles, though. And I'm even less sure as to when it might be "done".

As an aside are the also any plans to support Tab/Command completion in the CLI?

Have a look at https://github.com/spectreconsole/spectre.console/issues/267

nils-a avatar Feb 01 '22 07:02 nils-a

I recently encountered the need to support cancellation in my CLI, and wanted to share my solution/workaround.

In my case, SIGINT/SIGTERM would terminate the cli process, but some background work would continue running. The API managing this background work supported passing a CancellationToken, so my solution consist of a abstract implementation of AsyncCommand that uses a CancellationTokenSource to encapsulate the System.Console.CancelKeyPress and AppDomain.CurrentDomain.ProcessExit events:

using Spectre.Console.Cli;

namespace Cli;

public abstract class CancellableAsyncCommand : AsyncCommand
{
    public abstract Task<int> ExecuteAsync( CommandContext context, CancellationToken cancellation );

    public sealed override async Task<int> ExecuteAsync( CommandContext context )
    {
        using var cancellationSource = new CancellationTokenSource();

        System.Console.CancelKeyPress += onCancelKeyPress;
        AppDomain.CurrentDomain.ProcessExit += onProcessExit;

        using var _ = cancellationSource.Token.Register(
            ( ) =>
            {
                AppDomain.CurrentDomain.ProcessExit -= onProcessExit;
                System.Console.CancelKeyPress -= onCancelKeyPress;
            }
        );

        var cancellable = ExecuteAsync( context, cancellationSource.Token );
        return await cancellable;

        void onCancelKeyPress( object? sender, ConsoleCancelEventArgs e )
        {
            // NOTE: cancel event, don't terminate the process
            e.Cancel = true;

            cancellationSource.Cancel();
        }

        void onProcessExit( object? sender, EventArgs e )
        {
            if( cancellationSource.IsCancellationRequested )
            {
                // NOTE: SIGINT (cancel key was pressed, this shouldn't ever actually hit however, as we remove the event handler upon cancellation of the `cancellationSource`)
                return;
            }

            cancellationSource.Cancel();
        }
    }
}

When targeting NETCOREAPP directly a PosixSignalRegistration, as @Simonl9l suggests, makes for a simpler implementation:

using System.Runtime.InteropServices;
using Spectre.Console.Cli;

namespace Cli;

public abstract class CancellableAsyncCommand : AsyncCommand
{
    public abstract Task<int> ExecuteAsync( CommandContext context, CancellationToken cancellation );

    public sealed override async Task<int> ExecuteAsync( CommandContext context )
    {
        using var cancellationSource = new CancellationTokenSource();

        using var sigInt = PosixSignalRegistration.Create( PosixSignal.SIGINT, onSignal );
        using var sigQuit = PosixSignalRegistration.Create( PosixSignal.SIGQUIT, onSignal );
        using var sigTerm = PosixSignalRegistration.Create( PosixSignal.SIGTERM, onSignal );

        var cancellable = ExecuteAsync( context, cancellationSource.Token );
        return await cancellable;

        void onSignal( PosixSignalContext context )
        {
            context.Cancel = true;
            cancellationSource.Cancel();
        }
    }
}

With this approach, commands inherit from CancellableAsyncCommand, rather than AsyncCommand, and no changes should be needed to the Program.cs:

using Microsoft.Extensions.DependencyInjection;
using Spectre.Console.Cli;

var services = new ServiceCollection()
    .AddCliServices();

var registrar = new TypeRegistrar( services );
var app = new CommandApp<DefaultCommand>( registrar );

app.Configure(
    config =>
    {
#if DEBUG
        config.PropagateExceptions();
        config.ValidateExamples();
#endif
    }
);

return await app.RunAsync( args );

public class DefaultCommand : CancellableAsyncCommand
{
    public async Task<int> ExecuteAsync( CommandContext context, CancellationToken cancellation )
    {
        await DoWorkInThreads( cancellation );

        // ...
        return 0;
    } 
}

References:

P.S. I've just started using Spectre after using natemcmaster/CommandLineUtils for a while, and playing around with dotnet/command-line-api a bit; I think I'm in love.. ✌🏻❤️

Cryptoc1 avatar Feb 25 '22 05:02 Cryptoc1

@Cryptoc1 nice workaround, it feels it would be a nice and simple addition to the library (yet I'm sure it has some implications)

I've decouple the cancellation token source from the abstract command implementation so it can be used on both AsyncCommand and AsyncCommand<TSettings>, but otherwise pretty nice solution for now!

internal sealed class ConsoleAppCancellationTokenSource 
{
    private readonly CancellationTokenSource _cts = new();

    public CancellationToken Token => _cts.Token;

    public ConsoleAppCancellationTokenSource()
    {
        System.Console.CancelKeyPress += OnCancelKeyPress;
        AppDomain.CurrentDomain.ProcessExit += OnProcessExit;

        using var _ = _cts.Token.Register(
            () =>
            {
                AppDomain.CurrentDomain.ProcessExit -= OnProcessExit;
                System.Console.CancelKeyPress -= OnCancelKeyPress;
            }
        );
    }

    private void OnCancelKeyPress(object? sender, ConsoleCancelEventArgs e)
    {
        // NOTE: cancel event, don't terminate the process
        e.Cancel = true;

        _cts.Cancel();
    }

    private void OnProcessExit(object? sender, EventArgs e)
    {
        if (_cts.IsCancellationRequested)
        {
            // NOTE: SIGINT (cancel key was pressed, this shouldn't ever actually hit however, as we remove the event handler upon cancellation of the `cancellationSource`)
            return;
        }

        _cts.Cancel();
    }
}
public abstract class CancellableAsyncCommand : AsyncCommand
{
    private readonly ConsoleAppCancellationTokenSource _cancellationTokenSource = new();

    public abstract Task<int> ExecuteAsync(CommandContext context, CancellationToken cancellation);

    public sealed override async Task<int> ExecuteAsync(CommandContext context)
        => await ExecuteAsync(context, _cancellationTokenSource.Token);

}

public abstract class CancellableAsyncCommand<TSettings> : AsyncCommand<TSettings>
    where TSettings : CommandSettings
{
    private readonly ConsoleAppCancellationTokenSource _cancellationTokenSource = new();

    public abstract Task<int> ExecuteAsync(CommandContext context, TSettings settings, CancellationToken cancellation);

    public sealed override async Task<int> ExecuteAsync(CommandContext context, TSettings settings)
        => await ExecuteAsync(context, settings, _cancellationTokenSource.Token);
}

alvpickmans avatar Mar 29 '22 12:03 alvpickmans

Would be good to incorporate into core product, and also do simulate to create anValudateAsync path too.

Simonl9l avatar Mar 29 '22 15:03 Simonl9l

As another work-around is you could set up a ICommandInterceptor that adds a CancellationToken to the base settings if it is default. Then you have it in all your Settings parameters in every command.

solonrice avatar Jun 21 '22 23:06 solonrice

@patriksvensson do you need help with this issue? Think you'd like to move forward here?

codymullins avatar Jan 24 '24 15:01 codymullins

@Cryptoc1 Thank you for the CancellableAsyncCommand!

Cybrosys avatar Mar 25 '24 15:03 Cybrosys