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

Piping task timeout #243

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions CliWrap.Tests.Dummy/Commands/RunProcessCommand.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
using System.Diagnostics;
using System.Threading.Tasks;
using CliFx;
using CliFx.Attributes;
using CliFx.Infrastructure;

namespace CliWrap.Tests.Dummy.Commands;

[Command("run process")]
public class RunProcessCommand : ICommand
{
[CommandOption("path")]
public string FilePath { get; init; } = string.Empty;

[CommandOption("arguments")]
public string Arguments { get; init; } = string.Empty;

public ValueTask ExecuteAsync(IConsole console)
{
var startInfo = new ProcessStartInfo
{
FileName = FilePath,
Arguments = Arguments,
UseShellExecute = false,
CreateNoWindow = true
};

var process = new Process();
process.StartInfo = startInfo;
process.Start();

return default;
}
}
34 changes: 34 additions & 0 deletions CliWrap.Tests/CancellationSpecs.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Threading.Tasks;
using CliWrap.Buffered;
using CliWrap.EventStream;
using CliWrap.Exceptions;
using CliWrap.Tests.Utils;
using FluentAssertions;
using Xunit;
Expand Down Expand Up @@ -111,6 +112,39 @@ public async Task I_can_execute_a_command_with_buffering_and_cancel_it_immediate
ex.CancellationToken.Should().Be(cts.Token);
}

#if (NETCOREAPP || NETSTANDARD2_0_OR_GREATER)

[Fact(Timeout = 10000)]
public async Task I_can_execute_a_command_and_cancel_piping_when_command_is_finished_but_child_process_remains_running()
{
// Arrange
var cmd = Cli.Wrap(Dummy.Program.FilePath)
.WithArguments(
[
"run",
"process",
"--path",
Dummy.Program.FilePath,
"--arguments",
"sleep 00:00:15"
]
)
.WithStandardOutputPipe(PipeTarget.ToDelegate(_ => { }))
.WithStandardErrorPipe(PipeTarget.ToDelegate(_ => { }))
.WithPipingTimeout(TimeSpan.FromSeconds(1));

// Act
var executeAsync = async () => await cmd.ExecuteAsync();

// Assert
var ex = await Assert.ThrowsAnyAsync<PipesCancelledException>(
async () => await executeAsync()
);

ex.ExitCode.Should().Be(0);
}
#endif

[Fact(Timeout = 15000)]
public async Task I_can_execute_a_command_with_buffering_and_cancel_it_after_a_delay()
{
Expand Down
26 changes: 23 additions & 3 deletions CliWrap/Command.Execution.cs
Original file line number Diff line number Diff line change
Expand Up @@ -219,11 +219,14 @@ private async Task<CommandResult> ExecuteAsync(
.Register(process.Interrupt)
.ToAsyncDisposable();

using var pipeStdOutErrCts = CancellationTokenSource.CreateLinkedTokenSource(
forcefulCancellationToken
);
// Start piping streams in the background
var pipingTask = Task.WhenAll(
PipeStandardInputAsync(process, stdInCts.Token),
PipeStandardOutputAsync(process, forcefulCancellationToken),
PipeStandardErrorAsync(process, forcefulCancellationToken)
PipeStandardOutputAsync(process, pipeStdOutErrCts.Token),
PipeStandardErrorAsync(process, pipeStdOutErrCts.Token)
);

try
Expand All @@ -239,7 +242,13 @@ private async Task<CommandResult> ExecuteAsync(
// If the pipe is still trying to transfer data, this will cause it to abort.
await stdInCts.CancelAsync();

// Wait until piping is done and propagate exceptions
if (PipingTimeout != null)
{
// Cancel piping after specified time.
pipeStdOutErrCts.CancelAfter(PipingTimeout.Value);
}

// Wait until piping is done or cancelled and propagate exceptions
await pipingTask.ConfigureAwait(false);
}
// Swallow exceptions caused by internal and user-provided cancellations,
Expand All @@ -249,6 +258,7 @@ private async Task<CommandResult> ExecuteAsync(
|| ex.CancellationToken == gracefulCancellationToken
|| ex.CancellationToken == waitTimeoutCts.Token
|| ex.CancellationToken == stdInCts.Token
|| ex.CancellationToken == pipeStdOutErrCts.Token
) { }

// Throw if forceful cancellation was requested.
Expand All @@ -265,6 +275,16 @@ private async Task<CommandResult> ExecuteAsync(
+ $"Underlying process ({process.Name}#{process.Id}) was gracefully terminated."
);

if (pipeStdOutErrCts.IsCancellationRequested)
{
throw new PipesCancelledException(
process.ExitCode,
process.StartTime,
process.ExitTime,
$"Process ({process.Name}#{process.Id}) has exited, but piping was terminated due to timeout."
);
}

// Validate the exit code if required
if (process.ExitCode != 0 && Validation.IsZeroExitCodeValidationEnabled())
{
Expand Down
53 changes: 42 additions & 11 deletions CliWrap/Command.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ public partial class Command(
CommandResultValidation validation,
PipeSource standardInputPipe,
PipeTarget standardOutputPipe,
PipeTarget standardErrorPipe
PipeTarget standardErrorPipe,
TimeSpan? pipingTimeout
) : ICommandConfiguration
{
/// <summary>
Expand All @@ -35,7 +36,8 @@ public Command(string targetFilePath)
CommandResultValidation.ZeroExitCode,
PipeSource.Null,
PipeTarget.Null,
PipeTarget.Null
PipeTarget.Null,
null
) { }

/// <inheritdoc />
Expand All @@ -50,6 +52,9 @@ public Command(string targetFilePath)
/// <inheritdoc />
public Credentials Credentials { get; } = credentials;

/// <inheritdoc />
public TimeSpan? PipingTimeout { get; } = pipingTimeout;

/// <inheritdoc />
public IReadOnlyDictionary<string, string?> EnvironmentVariables { get; } =
environmentVariables;
Expand Down Expand Up @@ -80,7 +85,8 @@ public Command WithTargetFile(string targetFilePath) =>
Validation,
StandardInputPipe,
StandardOutputPipe,
StandardErrorPipe
StandardErrorPipe,
PipingTimeout
);

/// <summary>
Expand All @@ -101,7 +107,8 @@ public Command WithArguments(string arguments) =>
Validation,
StandardInputPipe,
StandardOutputPipe,
StandardErrorPipe
StandardErrorPipe,
PipingTimeout
);

/// <summary>
Expand Down Expand Up @@ -147,7 +154,25 @@ public Command WithWorkingDirectory(string workingDirPath) =>
Validation,
StandardInputPipe,
StandardOutputPipe,
StandardErrorPipe
StandardErrorPipe,
PipingTimeout
);

/// <summary>
/// Creates a copy of this command, setting the piping timeout to the specified value.
/// </summary>
public Command WithPipingTimeout(TimeSpan pipingTimeout) =>
new(
TargetFilePath,
Arguments,
WorkingDirPath,
Credentials,
EnvironmentVariables,
Validation,
StandardInputPipe,
StandardOutputPipe,
StandardErrorPipe,
pipingTimeout
);

/// <summary>
Expand All @@ -164,7 +189,8 @@ public Command WithCredentials(Credentials credentials) =>
Validation,
StandardInputPipe,
StandardOutputPipe,
StandardErrorPipe
StandardErrorPipe,
PipingTimeout
);

/// <summary>
Expand Down Expand Up @@ -196,7 +222,8 @@ public Command WithEnvironmentVariables(
Validation,
StandardInputPipe,
StandardOutputPipe,
StandardErrorPipe
StandardErrorPipe,
PipingTimeout
);

/// <summary>
Expand Down Expand Up @@ -226,7 +253,8 @@ public Command WithValidation(CommandResultValidation validation) =>
validation,
StandardInputPipe,
StandardOutputPipe,
StandardErrorPipe
StandardErrorPipe,
PipingTimeout
);

/// <summary>
Expand All @@ -243,7 +271,8 @@ public Command WithStandardInputPipe(PipeSource source) =>
Validation,
source,
StandardOutputPipe,
StandardErrorPipe
StandardErrorPipe,
PipingTimeout
);

/// <summary>
Expand All @@ -260,7 +289,8 @@ public Command WithStandardOutputPipe(PipeTarget target) =>
Validation,
StandardInputPipe,
target,
StandardErrorPipe
StandardErrorPipe,
PipingTimeout
);

/// <summary>
Expand All @@ -277,7 +307,8 @@ public Command WithStandardErrorPipe(PipeTarget target) =>
Validation,
StandardInputPipe,
StandardOutputPipe,
target
target,
PipingTimeout
);

/// <inheritdoc />
Expand Down
30 changes: 30 additions & 0 deletions CliWrap/Exceptions/PipesCancelledException.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
using System;
using System.Threading;

namespace CliWrap.Exceptions;

/// <summary>
/// The exception that is thrown when the pipes are cancelled.
/// </summary>
public class PipesCancelledException(
int exitCode,
DateTimeOffset startTime,
DateTimeOffset exitTime,
string message
) : OperationCanceledException(message)
{
/// <summary>
/// Exit code returned by the process.
/// </summary>
public int ExitCode { get; } = exitCode;

/// <summary>
/// Time when the process started.
/// </summary>
public DateTimeOffset StartTime { get; } = startTime;

/// <summary>
/// Time when the process exited.
/// </summary>
public DateTimeOffset ExitTime { get; } = exitTime;
}
8 changes: 7 additions & 1 deletion CliWrap/ICommandConfiguration.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Collections.Generic;
using System;
using System.Collections.Generic;

namespace CliWrap;

Expand Down Expand Up @@ -37,6 +38,11 @@ public interface ICommandConfiguration
/// </summary>
CommandResultValidation Validation { get; }

/// <summary>
/// How long to wait for piping to be finished after the process exits.
/// </summary>
TimeSpan? PipingTimeout { get; }

/// <summary>
/// Pipe source for the standard input stream of the underlying process.
/// </summary>
Expand Down