Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 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
1 change: 1 addition & 0 deletions src/Aspire.Cli/CliConfigNames.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ namespace Aspire.Cli;
internal static class CliConfigNames
{
public const string NoLogo = "ASPIRE_CLI_NOLOGO";
public const string AppHostStartupTimeout = "ASPIRE_CLI_START_TIMEOUT";
}
12 changes: 7 additions & 5 deletions src/Aspire.Cli/Commands/AppHostLauncher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ internal static void AddLaunchOptions(Command command)
/// <param name="isolated">Whether to run in isolated mode.</param>
/// <param name="isExtensionHost">Whether running inside VS Code extension.</param>
/// <param name="waitForDebugger">Whether the AppHost is waiting for a debugger to attach.</param>
/// <param name="timeoutSeconds">The maximum number of seconds to wait for AppHost startup.</param>
/// <param name="globalArgs">Global CLI args to forward to child process.</param>
/// <param name="additionalArgs">Additional unmatched args to forward.</param>
/// <param name="stopAfterLaunchDelay">Optional delay after launch before stopping the AppHost.</param>
Expand All @@ -94,6 +95,7 @@ public async Task<CommandResult> LaunchDetachedAsync(
bool isolated,
bool isExtensionHost,
bool waitForDebugger,
int timeoutSeconds,
IEnumerable<string> globalArgs,
IEnumerable<string> additionalArgs,
TimeSpan? stopAfterLaunchDelay,
Expand Down Expand Up @@ -165,7 +167,7 @@ public async Task<CommandResult> LaunchDetachedAsync(
{
launchResult = await interactionService.ShowStatusAsync(
RunCommandStrings.StartingAppHostInBackground,
() => LaunchAndWaitForBackchannelAsync(executablePath, childArgs, expectedHash, legacyHash, cancellationToken));
() => LaunchAndWaitForBackchannelAsync(executablePath, childArgs, expectedHash, legacyHash, TimeSpan.FromSeconds(timeoutSeconds), cancellationToken));
}
catch (OperationCanceledException)
{
Expand All @@ -175,7 +177,7 @@ public async Task<CommandResult> LaunchDetachedAsync(
// Handle failure cases
if (launchResult.Backchannel is null || launchResult.ChildProcess is null)
{
return HandleLaunchFailure(launchResult, childLogFile);
return HandleLaunchFailure(launchResult, childLogFile, timeoutSeconds);
}

// Display results
Expand Down Expand Up @@ -323,6 +325,7 @@ private async Task<LaunchResult> LaunchAndWaitForBackchannelAsync(
List<string> childArgs,
string expectedHash,
string? legacyHash,
TimeSpan timeout,
CancellationToken cancellationToken)
{
Process childProcess;
Expand Down Expand Up @@ -351,7 +354,6 @@ private async Task<LaunchResult> LaunchAndWaitForBackchannelAsync(
logger.LogDebug("Child CLI process started with PID: {PID}", childProcess.Id);

var startTime = timeProvider.GetUtcNow();
var timeout = TimeSpan.FromSeconds(120);
using var waitForBackchannelActivity = profilingTelemetry.StartDetachedWaitForBackchannel(childProcess.Id, expectedHash, legacyHash is not null);
var scanCount = 0;
IAppHostAuxiliaryBackchannel? connection = null;
Expand Down Expand Up @@ -626,7 +628,7 @@ private static void ObserveFaults(Task task)
TaskScheduler.Default);
}

private CommandResult HandleLaunchFailure(LaunchResult result, string childLogFile)
private CommandResult HandleLaunchFailure(LaunchResult result, string childLogFile, int timeoutSeconds)
{
if (result.ChildProcess is null)
{
Expand All @@ -646,7 +648,7 @@ private CommandResult HandleLaunchFailure(LaunchResult result, string childLogFi
}
else
{
failureMessage = RunCommandStrings.TimeoutWaitingForAppHost;
failureMessage = string.Format(CultureInfo.CurrentCulture, RunCommandStrings.TimeoutWaitingForAppHost, timeoutSeconds, CliConfigNames.AppHostStartupTimeout);
}

interactionService.DisplayError(RunCommandStrings.FailedToStartAppHost);
Expand Down
36 changes: 36 additions & 0 deletions src/Aspire.Cli/Commands/AppHostStartupTimeout.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Globalization;
using Aspire.Cli.Interaction;
using Aspire.Cli.Resources;
using Microsoft.Extensions.Configuration;

namespace Aspire.Cli.Commands;

internal static class AppHostStartupTimeout
{
public static bool TryGetTimeoutSeconds(IConfiguration configuration, IInteractionService interactionService, out int timeoutSeconds)
{
timeoutSeconds = WaitCommand.DefaultTimeoutSeconds;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default for AppHost startup is coupled to WaitCommand.DefaultTimeoutSeconds, but these are semantically distinct timeouts: WaitCommand.DefaultTimeoutSeconds is the default for aspire wait --timeout (waiting for a resource to reach a state), whereas this one controls how long aspire run / aspire start will wait for the AppHost to build + connect the backchannel. A future change to the wait command's default would silently shift AppHost startup behavior across the CLI. Consider a dedicated AppHostStartupTimeout.DefaultTimeoutSeconds so the two defaults can evolve independently.


var configuredTimeout = configuration[CliConfigNames.AppHostStartupTimeout];
if (string.IsNullOrWhiteSpace(configuredTimeout))
{
return true;
}

if (int.TryParse(configuredTimeout, NumberStyles.None, CultureInfo.InvariantCulture, out var parsedTimeout) &&
parsedTimeout > 0)
{
timeoutSeconds = parsedTimeout;
return true;
}

interactionService.DisplayError(string.Format(
CultureInfo.CurrentCulture,
RunCommandStrings.InvalidAppHostStartupTimeoutEnvironmentVariable,
CliConfigNames.AppHostStartupTimeout));
return false;
}
}
101 changes: 91 additions & 10 deletions src/Aspire.Cli/Commands/RunCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,11 @@ internal sealed class RunCommand : BaseCommand
private readonly FileLoggerProvider _fileLoggerProvider;
private readonly ICliHostEnvironment _hostEnvironment;
private readonly ProfilingTelemetry _profilingTelemetry;
private readonly TimeProvider _timeProvider;
private bool _isDetachMode;

private static readonly TimeSpan s_appHostStartupCancellationTimeout = TimeSpan.FromSeconds(5);

// Guest AppHosts can bring up the temporary server/backchannel and then fail immediately
// afterward when the guest startup process hits a syntax or pre-execute error. Keep the
// detached parent waiting briefly so those early failures are reported instead of hidden.
Expand Down Expand Up @@ -106,7 +109,8 @@ public RunCommand(
AppHostLauncher appHostLauncher,
FileLoggerProvider fileLoggerProvider,
ICliHostEnvironment hostEnvironment,
ProfilingTelemetry profilingTelemetry)
ProfilingTelemetry profilingTelemetry,
TimeProvider timeProvider)
: base("run", RunCommandStrings.Description, features, updateNotifier, executionContext, interactionService, telemetry)
{
_runner = runner;
Expand All @@ -122,6 +126,7 @@ public RunCommand(
_fileLoggerProvider = fileLoggerProvider;
_hostEnvironment = hostEnvironment;
_profilingTelemetry = profilingTelemetry;
_timeProvider = timeProvider;

Options.Add(s_detachOption);
Options.Add(s_noBuildOption);
Expand Down Expand Up @@ -167,10 +172,15 @@ protected override async Task<CommandResult> ExecuteAsync(ParseResult parseResul
return CommandResult.Failure(CliExitCodes.InvalidCommand, RunCommandStrings.NoBuildNotSupportedWithWatchMode);
}

if (!AppHostStartupTimeout.TryGetTimeoutSeconds(_configuration, InteractionService, out var timeoutSeconds))
{
return CommandResult.Failure(CliExitCodes.InvalidCommand);
}

// Handle detached mode - spawn child process and exit
if (detach)
{
return await ExecuteDetachedAsync(parseResult, passedAppHostProjectFile, isExtensionHost, cancellationToken);
return await ExecuteDetachedAsync(parseResult, passedAppHostProjectFile, isExtensionHost, timeoutSeconds, cancellationToken);
}

// A user may run `aspire run` in an Aspire terminal in VS Code. In this case, intercept and prompt
Expand Down Expand Up @@ -276,16 +286,29 @@ protected override async Task<CommandResult> ExecuteAsync(ParseResult parseResul

// Start the project run as a pending task - we'll handle UX while it runs
Task<int> pendingRun;
var startupTimeout = TimeSpan.FromSeconds(timeoutSeconds);
var startupStartTimestamp = _timeProvider.GetTimestamp();
using var runCancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the timeout path, if CancelAppHostStartupAsync exceeds its 5s grace it kicks off ObserveAppHostRunFailureAsync(pendingRun) as fire-and-forget and ExecuteAsync returns. The using var here then disposes the linked CTS while pendingRun is still running on a background task holding runCancellationTokenSource.Token. Operations against a disposed CTS's token (e.g. Register) throw ObjectDisposedException, which the observer will swallow at debug level — but the lifetime mismatch is worth either fixing (defer disposal until the observer completes) or annotating with a comment. Low-impact, lower confidence than the other two.

using (_profilingTelemetry.StartRunAppHostStartProject(project.LanguageId, noBuild, waitForDebugger))
{
pendingRun = project.RunAsync(context, cancellationToken);
pendingRun = project.RunAsync(context, runCancellationTokenSource.Token);
}

// Wait for the build to complete first (project handles its own build status spinners)
bool buildSuccess;
using (var waitForBuildActivity = _profilingTelemetry.StartRunAppHostWaitForBuild())
{
buildSuccess = await buildCompletionSource.Task.WaitAsync(cancellationToken);
try
{
buildSuccess = await buildCompletionSource.Task.WaitAsync(GetRemainingStartupTimeout(startupStartTimestamp, startupTimeout), _timeProvider, cancellationToken);
}
catch (TimeoutException)
{
runActivity?.SetTag(TelemetryConstants.Tags.ErrorType, "startup_timeout");
await CancelAppHostStartupAsync(runCancellationTokenSource, pendingRun).ConfigureAwait(false);
return CreateStartupTimeoutResult(timeoutSeconds);
}

waitForBuildActivity.SetAppHostBuildSuccess(buildSuccess);
}
if (!buildSuccess)
Expand All @@ -309,9 +332,19 @@ protected override async Task<CommandResult> ExecuteAsync(ParseResult parseResul
IAppHostCliBackchannel backchannel;
using (var waitForBackchannelActivity = _profilingTelemetry.StartRunAppHostWaitForBackchannel())
{
backchannel = await InteractionService.ShowStatusAsync(
RunCommandStrings.ConnectingToAppHost,
async () => await backchannelCompletionSource.Task.WaitAsync(cancellationToken));
try
{
backchannel = await InteractionService.ShowStatusAsync(
RunCommandStrings.ConnectingToAppHost,
async () => await backchannelCompletionSource.Task.WaitAsync(GetRemainingStartupTimeout(startupStartTimestamp, startupTimeout), _timeProvider, cancellationToken));
}
catch (TimeoutException)
{
runActivity?.SetTag(TelemetryConstants.Tags.ErrorType, "startup_timeout");
await CancelAppHostStartupAsync(runCancellationTokenSource, pendingRun).ConfigureAwait(false);
return CreateStartupTimeoutResult(timeoutSeconds);
}

waitForBackchannelActivity.SetAppHostBackchannelConnected(true);
}

Expand Down Expand Up @@ -485,7 +518,7 @@ await InteractionService.DisplayLiveAsync(BuildLiveRenderable(), async updateTar
: CommandResult.FromExitCode(exitCode);
}
}
catch (OperationCanceledException ex) when (ex.CancellationToken == cancellationToken || ex is ExtensionOperationCanceledException)
catch (OperationCanceledException ex) when (cancellationToken.IsCancellationRequested || ex.CancellationToken == cancellationToken || ex is ExtensionOperationCanceledException)
{
runActivity?.SetTag(TelemetryConstants.Tags.ErrorType, "canceled");

Expand Down Expand Up @@ -798,12 +831,12 @@ public void ProcessResourceState(RpcResourceState resourceState, Action<string,
/// with the poll delay. Shows exit code and log file path.
/// Returns <see cref="CliExitCodes.FailedToDotnetRunAppHost"/>.</item>
/// <item><b>Timeout waiting for backchannel</b>: The auxiliary backchannel socket doesn't appear
/// within 120 seconds. The child process is killed. Shows timeout message and log file path.
/// within the configured startup timeout. The child process is killed. Shows timeout message and log file path.
/// Returns <see cref="CliExitCodes.FailedToDotnetRunAppHost"/>.</item>
/// </list>
/// <para>On any failure, the log file path is displayed so the user can investigate.</para>
/// </remarks>
private Task<CommandResult> ExecuteDetachedAsync(ParseResult parseResult, FileInfo? passedAppHostProjectFile, bool isExtensionHost, CancellationToken cancellationToken)
private Task<CommandResult> ExecuteDetachedAsync(ParseResult parseResult, FileInfo? passedAppHostProjectFile, bool isExtensionHost, int timeoutSeconds, CancellationToken cancellationToken)
{
var format = parseResult.GetValue(AppHostLauncher.s_formatOption);
var isolated = parseResult.GetValue(AppHostLauncher.s_isolatedOption);
Expand All @@ -827,10 +860,58 @@ private Task<CommandResult> ExecuteDetachedAsync(ParseResult parseResult, FileIn
isolated,
isExtensionHost,
waitForDebugger,
timeoutSeconds,
globalArgs,
additionalArgs,
stopAfterLaunchDelay,
cancellationToken);
}

private TimeSpan GetRemainingStartupTimeout(long startupStartTimestamp, TimeSpan startupTimeout)
{
var elapsed = _timeProvider.GetElapsedTime(startupStartTimestamp);
return elapsed >= startupTimeout ? TimeSpan.Zero : startupTimeout - elapsed;
}

private async Task CancelAppHostStartupAsync(CancellationTokenSource runCancellationTokenSource, Task<int> pendingRun)
{
runCancellationTokenSource.Cancel();

try
{
await pendingRun.WaitAsync(s_appHostStartupCancellationTimeout, _timeProvider).ConfigureAwait(false);
}
catch (OperationCanceledException) when (runCancellationTokenSource.IsCancellationRequested)
{
}
catch (TimeoutException ex)
{
_logger.LogDebug(ex, "Timed out waiting for AppHost startup cancellation to complete.");
_ = ObserveAppHostRunFailureAsync(pendingRun);
}
catch (Exception ex)
{
_logger.LogDebug(ex, "AppHost run failed after startup cancellation.");
}
}

private async Task ObserveAppHostRunFailureAsync(Task<int> pendingRun)
{
try
{
await pendingRun.ConfigureAwait(false);
}
catch (Exception ex)
{
_logger.LogDebug(ex, "AppHost run failed after startup cancellation timeout.");
}
}

private static CommandResult CreateStartupTimeoutResult(int timeoutSeconds)
{
return CommandResult.Failure(
CliExitCodes.FailedToDotnetRunAppHost,
string.Format(CultureInfo.CurrentCulture, RunCommandStrings.TimeoutWaitingForAppHost, timeoutSeconds, CliConfigNames.AppHostStartupTimeout));
}

}
6 changes: 6 additions & 0 deletions src/Aspire.Cli/Commands/StartCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,18 @@ await extensionInteractionService.StartDebugSessionAsync(
additionalArgs.Add("--no-build");
}

if (!AppHostStartupTimeout.TryGetTimeoutSeconds(_configuration, InteractionService, out var timeoutSeconds))
{
return CommandResult.Failure(CliExitCodes.InvalidCommand);
}

return await _appHostLauncher.LaunchDetachedAsync(
passedAppHostProjectFile,
format,
isolated,
isExtensionHost,
waitForDebugger,
timeoutSeconds,
globalArgs,
additionalArgs,
stopAfterLaunchDelay,
Expand Down
4 changes: 3 additions & 1 deletion src/Aspire.Cli/Commands/WaitCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@ internal sealed class WaitCommand : BaseCommand
DefaultValueFactory = _ => "healthy"
};

internal const int DefaultTimeoutSeconds = 120;

private static readonly Option<int> s_timeoutOption = new("--timeout")
{
Description = WaitCommandStrings.TimeoutOptionDescription,
DefaultValueFactory = _ => 120
DefaultValueFactory = _ => DefaultTimeoutSeconds
};

private static readonly OptionWithLegacy<FileInfo?> s_appHostOption = new("--apphost", "--project", SharedCommandStrings.AppHostOptionDescription);
Expand Down
27 changes: 26 additions & 1 deletion src/Aspire.Cli/DotNet/ProcessExecution.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,20 @@ public async Task<int> WaitForExitAsync(CancellationToken cancellationToken)
{
_logger.LogDebug("{FileName}({ProcessId}) waiting for exit", FileName, _process.Id);

await _process.WaitForExitAsync(cancellationToken);
try
{
await _process.WaitForExitAsync(cancellationToken);
}
catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
{
if (!_process.HasExited)
{
_logger.LogDebug("{FileName}({ProcessId}) wait was canceled, killing it", FileName, _process.Id);
TryKillProcessTree();
}

throw;
}

if (!_process.HasExited)
{
Expand Down Expand Up @@ -204,4 +217,16 @@ private void RecordForwarderActivity()
{
Interlocked.Exchange(ref _lastForwarderActivityTimestamp, Stopwatch.GetTimestamp());
}

private void TryKillProcessTree()
{
try
{
_process.Kill(entireProcessTree: true);
}
catch (Exception ex)
{
_logger.LogDebug(ex, "{FileName}({ProcessId}) failed to kill process tree after wait cancellation", FileName, _process.Id);
}
}
}
6 changes: 6 additions & 0 deletions src/Aspire.Cli/Resources/RunCommandStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Aspire.Cli/Resources/RunCommandStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,12 @@
<value>AppHost failed to build.</value>
</data>
<data name="TimeoutWaitingForAppHost" xml:space="preserve">
<value>Timeout waiting for AppHost to start.</value>
<value>Timed out waiting {0}s for AppHost to start. If the AppHost is still building or starting, set {1} to a higher value and try again.</value>
<comment>Do not localize the {1} placeholder. It is an environment variable name.</comment>
</data>
<data name="InvalidAppHostStartupTimeoutEnvironmentVariable" xml:space="preserve">
<value>The {0} environment variable must be a positive number of seconds.</value>
<comment>Do not localize the {0} placeholder. It is an environment variable name.</comment>
</data>
<data name="CheckLogsForDetails" xml:space="preserve">
<value>Check logs for details: {0}</value>
Expand Down
Loading
Loading