Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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
32 changes: 20 additions & 12 deletions src/Aspire.Cli/Commands/AppHostLauncher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.CommandLine;
using System.Diagnostics;
using System.Globalization;
using System.Text.Json;
using Aspire.Cli.Backchannel;
Expand All @@ -27,6 +26,7 @@ internal sealed class AppHostLauncher(
CliExecutionContext executionContext,
IInteractionService interactionService,
IAuxiliaryBackchannelMonitor backchannelMonitor,
IDetachedProcessLauncher detachedProcessLauncher,
ICliHostEnvironment hostEnvironment,
AspireCliTelemetry telemetry,
ILogger<AppHostLauncher> logger,
Expand Down Expand Up @@ -54,15 +54,21 @@ internal sealed class AppHostLauncher(
Description = SharedCommandStrings.IsolatedOptionDescription
};

internal static readonly Option<int> s_timeoutOption = WaitCommand.CreateTimeoutOption();

/// <summary>
/// Adds the detached launch options to a command so they appear in --help.
/// Called by both RunCommand and StartCommand to keep options in sync.
/// Adds the shared AppHost launch options to a command so they appear in --help.
/// Called by both RunCommand and StartCommand to keep shared options in sync.
/// </summary>
internal static void AddLaunchOptions(Command command)
internal static void AddLaunchOptions(Command command, bool includeTimeout = false)
{
command.Options.Add(s_appHostOption);
command.Options.Add(s_formatOption);
command.Options.Add(s_isolatedOption);
if (includeTimeout)
{
command.Options.Add(s_timeoutOption);
}
}

/// <summary>
Expand All @@ -73,6 +79,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 the AppHost backchannel.</param>
/// <param name="globalArgs">Global CLI args to forward to child process.</param>
/// <param name="additionalArgs">Additional unmatched args to forward.</param>
/// <param name="cancellationToken">Cancellation token.</param>
Expand All @@ -83,6 +90,7 @@ public async Task<int> LaunchDetachedAsync(
bool isolated,
bool isExtensionHost,
bool waitForDebugger,
int timeoutSeconds,
IEnumerable<string> globalArgs,
IEnumerable<string> additionalArgs,
CancellationToken cancellationToken)
Expand Down Expand Up @@ -149,12 +157,12 @@ public async Task<int> LaunchDetachedAsync(
// Start the child process and wait for the backchannel
var launchResult = await interactionService.ShowStatusAsync(
RunCommandStrings.StartingAppHostInBackground,
() => LaunchAndWaitForBackchannelAsync(executablePath, childArgs, expectedHash, legacyHash, cancellationToken));
() => LaunchAndWaitForBackchannelAsync(executablePath, childArgs, expectedHash, legacyHash, TimeSpan.FromSeconds(timeoutSeconds), cancellationToken));

// 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 @@ -245,20 +253,21 @@ private async Task StopExistingInstancesAsync(FileInfo effectiveAppHostFile, Can
internal static bool IsExtensionEnvironmentVariable(string name) =>
name.StartsWith(ExtensionEnvironmentVariablePrefix, StringComparison.OrdinalIgnoreCase);

private record LaunchResult(Process? ChildProcess, IAppHostAuxiliaryBackchannel? Backchannel, DashboardUrlsState? DashboardUrls, bool ChildExitedEarly, int ChildExitCode);
private record LaunchResult(IDetachedProcess? ChildProcess, IAppHostAuxiliaryBackchannel? Backchannel, DashboardUrlsState? DashboardUrls, bool ChildExitedEarly, int ChildExitCode);

private async Task<LaunchResult> LaunchAndWaitForBackchannelAsync(
string executablePath,
List<string> childArgs,
string expectedHash,
string? legacyHash,
TimeSpan timeout,
CancellationToken cancellationToken)
{
Process childProcess;
IDetachedProcess childProcess;

try
{
childProcess = DetachedProcessLauncher.Start(
childProcess = detachedProcessLauncher.Start(
executablePath,
childArgs,
executionContext.WorkingDirectory.FullName,
Expand All @@ -274,7 +283,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);

while (timeProvider.GetUtcNow() - startTime < timeout)
{
Expand Down Expand Up @@ -319,7 +327,7 @@ private async Task<LaunchResult> LaunchAndWaitForBackchannelAsync(
return new LaunchResult(childProcess, null, null, false, 0);
}

private int HandleLaunchFailure(LaunchResult result, string childLogFile)
private int HandleLaunchFailure(LaunchResult result, string childLogFile, int timeoutSeconds)
{
if (result.ChildProcess is null)
{
Expand All @@ -333,7 +341,7 @@ private int HandleLaunchFailure(LaunchResult result, string childLogFile)
}
else
{
interactionService.DisplayError(RunCommandStrings.TimeoutWaitingForAppHost);
interactionService.DisplayError(string.Format(CultureInfo.CurrentCulture, RunCommandStrings.TimeoutWaitingForAppHost, timeoutSeconds));

if (!result.ChildProcess.HasExited)
{
Expand Down
64 changes: 54 additions & 10 deletions src/Aspire.Cli/Commands/RunCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public RunCommand(

Options.Add(s_detachOption);
Options.Add(s_noBuildOption);
AppHostLauncher.AddLaunchOptions(this);
AppHostLauncher.AddLaunchOptions(this, includeTimeout: true);

if (ExtensionHelper.IsExtensionHost(InteractionService, out _, out _))
{
Expand All @@ -137,6 +137,7 @@ protected override async Task<int> ExecuteAsync(ParseResult parseResult, Cancell
var noBuild = parseResult.GetValue(s_noBuildOption);
var format = parseResult.GetValue(AppHostLauncher.s_formatOption);
var isolated = parseResult.GetValue(AppHostLauncher.s_isolatedOption);
var timeoutSeconds = parseResult.GetValue(AppHostLauncher.s_timeoutOption);
var isExtensionHost = ExtensionHelper.IsExtensionHost(InteractionService, out _, out _);
var startDebugSession = false;
if (isExtensionHost)
Expand All @@ -152,6 +153,11 @@ protected override async Task<int> ExecuteAsync(ParseResult parseResult, Cancell
return ExitCodeConstants.InvalidCommand;
}

if (!WaitCommand.ValidateTimeout(timeoutSeconds, InteractionService))
{
return ExitCodeConstants.InvalidCommand;
}

// Validate that --no-build is not used when watch mode would be enabled
// Watch mode is enabled when DefaultWatchEnabled feature is true, or when running under extension host (not in debug session)
var watchModeEnabled = _features.IsFeatureEnabled(KnownFeatures.DefaultWatchEnabled, defaultValue: false) || (isExtensionHost && !startDebugSession);
Expand All @@ -164,7 +170,7 @@ protected override async Task<int> ExecuteAsync(ParseResult parseResult, Cancell
// 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 @@ -255,11 +261,26 @@ protected override async Task<int> ExecuteAsync(ParseResult parseResult, Cancell
BackchannelCompletionSource = backchannelCompletionSource,
};

var startupTimeout = TimeSpan.FromSeconds(timeoutSeconds);
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.


// Start the project run as a pending task - we'll handle UX while it runs
var pendingRun = project.RunAsync(context, cancellationToken);
var pendingRun = project.RunAsync(context, runCancellationTokenSource.Token);

// Wait for the build to complete first (project handles its own build status spinners)
var buildSuccess = await buildCompletionSource.Task.WaitAsync(cancellationToken);
bool buildSuccess;
try
{
buildSuccess = await buildCompletionSource.Task.WaitAsync(startupTimeout, cancellationToken);
}
Comment thread
sebastienros marked this conversation as resolved.
Outdated
catch (TimeoutException)
{
runActivity?.SetTag(TelemetryConstants.Tags.ErrorType, "startup_timeout");
CancelAppHostStartup(runCancellationTokenSource);
Comment thread
sebastienros marked this conversation as resolved.
Outdated
DisplayStartupTimeout(timeoutSeconds);
return ExitCodeConstants.FailedToDotnetRunAppHost;
}

if (!buildSuccess)
{
runActivity?.SetTag(TelemetryConstants.Tags.ErrorType, "build_failed");
Expand All @@ -279,9 +300,20 @@ protected override async Task<int> ExecuteAsync(ParseResult parseResult, Cancell
}

// Now wait for the backchannel to be established
var backchannel = await InteractionService.ShowStatusAsync(
RunCommandStrings.ConnectingToAppHost,
async () => await backchannelCompletionSource.Task.WaitAsync(cancellationToken));
IAppHostCliBackchannel backchannel;
try
{
backchannel = await InteractionService.ShowStatusAsync(
RunCommandStrings.ConnectingToAppHost,
async () => await backchannelCompletionSource.Task.WaitAsync(startupTimeout, cancellationToken));
}
catch (TimeoutException)
{
runActivity?.SetTag(TelemetryConstants.Tags.ErrorType, "startup_timeout");
CancelAppHostStartup(runCancellationTokenSource);
DisplayStartupTimeout(timeoutSeconds);
return ExitCodeConstants.FailedToDotnetRunAppHost;
}

// Set up log capture - writes to unified CLI log file
var pendingLogCapture = CaptureAppHostLogsAsync(_fileLoggerProvider, backchannel, _interactionService, cancellationToken);
Expand Down Expand Up @@ -392,7 +424,7 @@ await InteractionService.DisplayLiveAsync(BuildLiveRenderable(), async updateTar
await pendingLogCapture;
return await pendingRun;
}
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");
InteractionService.DisplayCancellationMessage();
Expand Down Expand Up @@ -648,12 +680,12 @@ public void ProcessResourceState(RpcResourceState resourceState, Action<string,
/// with the poll delay. Shows exit code and log file path.
/// Returns <see cref="ExitCodeConstants.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 timeout. The child process is killed. Shows timeout message and log file path.
/// Returns <see cref="ExitCodeConstants.FailedToDotnetRunAppHost"/>.</item>
/// </list>
/// <para>On any failure, the log file path is displayed so the user can investigate.</para>
/// </remarks>
private Task<int> ExecuteDetachedAsync(ParseResult parseResult, FileInfo? passedAppHostProjectFile, bool isExtensionHost, CancellationToken cancellationToken)
private Task<int> 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 @@ -673,9 +705,21 @@ private Task<int> ExecuteDetachedAsync(ParseResult parseResult, FileInfo? passed
isolated,
isExtensionHost,
waitForDebugger,
timeoutSeconds,
globalArgs,
additionalArgs,
cancellationToken);
}

private static void CancelAppHostStartup(CancellationTokenSource runCancellationTokenSource)
{
runCancellationTokenSource.Cancel();
}

private void DisplayStartupTimeout(int timeoutSeconds)
{
InteractionService.DisplayError(string.Format(CultureInfo.CurrentCulture, RunCommandStrings.TimeoutWaitingForAppHost, timeoutSeconds));
InteractionService.DisplayMessage(KnownEmojis.PageFacingUp, string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.SeeLogsAt, ExecutionContext.LogFilePath));
}

}
9 changes: 8 additions & 1 deletion src/Aspire.Cli/Commands/StartCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public StartCommand(
_appHostLauncher = appHostLauncher;

Options.Add(s_noBuildOption);
AppHostLauncher.AddLaunchOptions(this);
AppHostLauncher.AddLaunchOptions(this, includeTimeout: true);

TreatUnmatchedTokensAsErrors = false;
}
Expand All @@ -44,6 +44,7 @@ protected override async Task<int> ExecuteAsync(ParseResult parseResult, Cancell
var passedAppHostProjectFile = parseResult.GetValue(AppHostLauncher.s_appHostOption);
var format = parseResult.GetValue(AppHostLauncher.s_formatOption);
var isolated = parseResult.GetValue(AppHostLauncher.s_isolatedOption);
var timeoutSeconds = parseResult.GetValue(AppHostLauncher.s_timeoutOption);

var noBuild = parseResult.GetValue(s_noBuildOption);
// `aspire start` is always user-initiated — the VS Code extension only invokes
Expand All @@ -59,12 +60,18 @@ protected override async Task<int> ExecuteAsync(ParseResult parseResult, Cancell
additionalArgs.Add("--no-build");
}

if (!WaitCommand.ValidateTimeout(timeoutSeconds, InteractionService))
{
return ExitCodeConstants.InvalidCommand;
}

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

private static readonly Option<int> s_timeoutOption = new("--timeout")
internal const int DefaultTimeoutSeconds = 120;

private static readonly Option<int> s_timeoutOption = CreateTimeoutOption();

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

internal static bool ValidateTimeout(int timeoutSeconds, IInteractionService interactionService)
{
if (timeoutSeconds > 0)
{
return true;
}

interactionService.DisplayError(WaitCommandStrings.TimeoutMustBePositive);
return false;
}

private static readonly OptionWithLegacy<FileInfo?> s_appHostOption = new("--apphost", "--project", SharedCommandStrings.AppHostOptionDescription);

public WaitCommand(
Expand Down Expand Up @@ -81,10 +96,8 @@ protected override async Task<int> ExecuteAsync(ParseResult parseResult, Cancell
return ExitCodeConstants.InvalidCommand;
}

// Validate timeout
if (timeoutSeconds <= 0)
if (!ValidateTimeout(timeoutSeconds, _interactionService))
{
_interactionService.DisplayError(WaitCommandStrings.TimeoutMustBePositive);
return ExitCodeConstants.InvalidCommand;
}

Expand Down
15 changes: 14 additions & 1 deletion src/Aspire.Cli/DotNet/ProcessExecution.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,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);
_process.Kill(entireProcessTree: true);
Comment thread
sebastienros marked this conversation as resolved.
Outdated
}

throw;
}

if (!_process.HasExited)
{
Expand Down
Loading
Loading