Skip to content

Commit 806a6d9

Browse files
authored
Add new SetAction overload to avoid fire & forget silent error (#2565)
* add failing test * introduce a new overload that will be selected by the compiler hide it from intellisense, so the users are more likely to use the overload that takes CancellationToken
1 parent 3bbc68e commit 806a6d9

File tree

3 files changed

+46
-1
lines changed

3 files changed

+46
-1
lines changed

src/System.CommandLine.ApiCompatibility.Tests/ApiCompatibilityApprovalTests.System_CommandLine_api_is_not_changed.approved.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
public System.Void SetAction(System.Action<ParseResult> action)
5555
public System.Void SetAction(System.Func<ParseResult,System.Int32> action)
5656
public System.Void SetAction(System.Func<ParseResult,System.Threading.CancellationToken,System.Threading.Tasks.Task> action)
57+
public System.Void SetAction(System.Func<ParseResult,System.Threading.Tasks.Task> action)
5758
public System.Void SetAction(System.Func<ParseResult,System.Threading.CancellationToken,System.Threading.Tasks.Task<System.Int32>> action)
5859
public class CommandLineConfiguration
5960
.ctor(Command rootCommand)

src/System.CommandLine.Tests/Invocation/InvocationTests.cs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,27 @@ public async Task Anonymous_RootCommand_int_returning_Action_can_set_custom_resu
188188

189189
(await rootCommand.Parse("").InvokeAsync()).Should().Be(123);
190190
}
191-
191+
192+
[Fact] // https://github.com/dotnet/command-line-api/issues/2562
193+
public void Anonymous_async_action_is_not_mapped_into_sync_void_with_fire_and_forget()
194+
{
195+
RootCommand rootCommand = new();
196+
using CancellationTokenSource cts = new();
197+
Task delay = Task.Delay(TimeSpan.FromHours(1), cts.Token);
198+
199+
rootCommand.SetAction(async parseResult =>
200+
{
201+
await delay;
202+
});
203+
204+
Task started = rootCommand.Parse("").InvokeAsync();
205+
206+
// The action is supposed to wait for an hour, so it should not complete immediately.
207+
started.IsCompleted.Should().BeFalse();
208+
209+
cts.Cancel();
210+
}
211+
192212
[Fact]
193213
public void Terminating_option_action_short_circuits_command_action()
194214
{

src/System.CommandLine/Command.cs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,30 @@ public void SetAction(Func<ParseResult, CancellationToken, Task> action)
149149
});
150150
}
151151

152+
/// <summary>
153+
/// Sets an asynchronous action to be run when the command is invoked.
154+
/// </summary>
155+
/// <remarks>
156+
/// When possible, prefer using the <see cref="SetAction(Func{ParseResult, CancellationToken, Task})"/> overload
157+
/// and passing the <see cref="CancellationToken"/> parameter to the async method(s) called by the action.
158+
/// </remarks>
159+
// Hide from intellisense, it's public to avoid the compiler choosing a sync overload
160+
// for an async action (and fire and forget issue described in https://github.com/dotnet/command-line-api/issues/2562).
161+
[EditorBrowsable(EditorBrowsableState.Never)]
162+
public void SetAction(Func<ParseResult, Task> action)
163+
{
164+
if (action is null)
165+
{
166+
throw new ArgumentNullException(nameof(action));
167+
}
168+
169+
Action = new AnonymousAsynchronousCommandLineAction(async (context, cancellationToken) =>
170+
{
171+
await action(context);
172+
return 0;
173+
});
174+
}
175+
152176
/// <summary>
153177
/// Sets an asynchronous action when the command is invoked.
154178
/// </summary>

0 commit comments

Comments
 (0)