-
Notifications
You must be signed in to change notification settings - Fork 800
Improve detach mode: fix pipe handle inheritance and unify log naming #14424
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -82,6 +82,11 @@ internal sealed class RunCommand : BaseCommand | |||||||||||||||||||||
| { | ||||||||||||||||||||||
| Description = RunCommandStrings.IsolatedArgumentDescription | ||||||||||||||||||||||
| }; | ||||||||||||||||||||||
| private static readonly Option<string?> s_logFileOption = new("--log-file") | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| Description = "Path to write the log file (used internally by --detach).", | ||||||||||||||||||||||
| Hidden = true | ||||||||||||||||||||||
| }; | ||||||||||||||||||||||
| private readonly Option<bool>? _startDebugSessionOption; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| public RunCommand( | ||||||||||||||||||||||
|
|
@@ -121,6 +126,7 @@ public RunCommand( | |||||||||||||||||||||
| Options.Add(s_detachOption); | ||||||||||||||||||||||
| Options.Add(s_formatOption); | ||||||||||||||||||||||
| Options.Add(s_isolatedOption); | ||||||||||||||||||||||
| Options.Add(s_logFileOption); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (ExtensionHelper.IsExtensionHost(InteractionService, out _, out _)) | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
|
|
@@ -278,9 +284,9 @@ protected override async Task<int> ExecuteAsync(ParseResult parseResult, Cancell | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Handle remote environments (Codespaces, Remote Containers, SSH) | ||||||||||||||||||||||
| var isCodespaces = dashboardUrls.CodespacesUrlWithLoginToken is not null; | ||||||||||||||||||||||
| var isRemoteContainers = _configuration.GetValue<bool>("REMOTE_CONTAINERS", false); | ||||||||||||||||||||||
| var isSshRemote = _configuration.GetValue<string?>("VSCODE_IPC_HOOK_CLI") is not null | ||||||||||||||||||||||
| && _configuration.GetValue<string?>("SSH_CONNECTION") is not null; | ||||||||||||||||||||||
| var isRemoteContainers = string.Equals(_configuration["REMOTE_CONTAINERS"], "true", StringComparison.OrdinalIgnoreCase); | ||||||||||||||||||||||
| var isSshRemote = _configuration["VSCODE_IPC_HOOK_CLI"] is not null | ||||||||||||||||||||||
| && _configuration["SSH_CONNECTION"] is not null; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| AppendCtrlCMessage(longestLocalizedLengthWithColon); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
|
@@ -476,7 +482,7 @@ internal static int RenderAppHostSummary( | |||||||||||||||||||||
| new Align(new Markup($"[bold green]{dashboardLabel}[/]:"), HorizontalAlignment.Right), | ||||||||||||||||||||||
| new Markup("[dim]N/A[/]")); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| grid.AddRow(Text.Empty, Text.Empty); | ||||||||||||||||||||||
| grid.AddRow(Text.Empty, Text.Empty); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Logs row | ||||||||||||||||||||||
|
|
@@ -639,18 +645,23 @@ private async Task<int> ExecuteDetachedAsync(ParseResult parseResult, FileInfo? | |||||||||||||||||||||
| _logger.LogDebug("Found {Count} running instance(s) for this AppHost, stopping them first", existingSockets.Length); | ||||||||||||||||||||||
| var manager = new RunningInstanceManager(_logger, _interactionService, _timeProvider); | ||||||||||||||||||||||
| // Stop all running instances in parallel - don't block on failures | ||||||||||||||||||||||
| var stopTasks = existingSockets.Select(socket => | ||||||||||||||||||||||
| var stopTasks = existingSockets.Select(socket => | ||||||||||||||||||||||
| manager.StopRunningInstanceAsync(socket, cancellationToken)); | ||||||||||||||||||||||
| await Task.WhenAll(stopTasks).ConfigureAwait(false); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Build the arguments for the child CLI process | ||||||||||||||||||||||
| // Tell the child where to write its log so we can find it on failure. | ||||||||||||||||||||||
| var childLogFile = GenerateChildLogFilePath(); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| var args = new List<string> | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| "run", | ||||||||||||||||||||||
| "--non-interactive", | ||||||||||||||||||||||
| "--project", | ||||||||||||||||||||||
| effectiveAppHostFile.FullName | ||||||||||||||||||||||
| effectiveAppHostFile.FullName, | ||||||||||||||||||||||
| "--log-file", | ||||||||||||||||||||||
| childLogFile | ||||||||||||||||||||||
| }; | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Pass through global options that should be forwarded to child CLI | ||||||||||||||||||||||
|
|
@@ -687,14 +698,17 @@ private async Task<int> ExecuteDetachedAsync(ParseResult parseResult, FileInfo? | |||||||||||||||||||||
| dotnetPath, isDotnetHost, string.Join(" ", args)); | ||||||||||||||||||||||
| _logger.LogDebug("Working directory: {WorkingDirectory}", ExecutionContext.WorkingDirectory.FullName); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Redirect stdout/stderr to suppress child output - it writes to log file anyway | ||||||||||||||||||||||
| // Don't redirect stdout/stderr - child writes to log file anyway. | ||||||||||||||||||||||
| // Redirecting creates pipe handles that get inherited by the AppHost grandchild, | ||||||||||||||||||||||
| // which prevents callers using synchronous process APIs (e.g. execSync) from | ||||||||||||||||||||||
| // detecting that the CLI has exited, since the pipe stays open until the AppHost dies. | ||||||||||||||||||||||
| var startInfo = new ProcessStartInfo | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| FileName = dotnetPath, | ||||||||||||||||||||||
| UseShellExecute = false, | ||||||||||||||||||||||
| CreateNoWindow = true, | ||||||||||||||||||||||
| RedirectStandardOutput = true, | ||||||||||||||||||||||
| RedirectStandardError = true, | ||||||||||||||||||||||
| RedirectStandardOutput = false, | ||||||||||||||||||||||
| RedirectStandardError = false, | ||||||||||||||||||||||
| RedirectStandardInput = false, | ||||||||||||||||||||||
| WorkingDirectory = ExecutionContext.WorkingDirectory.FullName | ||||||||||||||||||||||
| }; | ||||||||||||||||||||||
|
|
@@ -727,24 +741,6 @@ private async Task<int> ExecuteDetachedAsync(ParseResult parseResult, FileInfo? | |||||||||||||||||||||
| return null; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Start async reading of stdout/stderr to prevent buffer blocking | ||||||||||||||||||||||
| // Log output for debugging purposes | ||||||||||||||||||||||
| childProcess.OutputDataReceived += (_, e) => | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| if (e.Data is not null) | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| _logger.LogDebug("Child stdout: {Line}", e.Data); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| }; | ||||||||||||||||||||||
| childProcess.ErrorDataReceived += (_, e) => | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| if (e.Data is not null) | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| _logger.LogDebug("Child stderr: {Line}", e.Data); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| }; | ||||||||||||||||||||||
| childProcess.BeginOutputReadLine(); | ||||||||||||||||||||||
| childProcess.BeginErrorReadLine(); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| catch (Exception ex) | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
|
|
@@ -823,10 +819,13 @@ private async Task<int> ExecuteDetachedAsync(ParseResult parseResult, FileInfo? | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| if (childExitedEarly) | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| _interactionService.DisplayError(string.Format( | ||||||||||||||||||||||
| CultureInfo.CurrentCulture, | ||||||||||||||||||||||
| RunCommandStrings.AppHostExitedWithCode, | ||||||||||||||||||||||
| childExitCode)); | ||||||||||||||||||||||
| // Show a friendly message based on well-known exit codes from the child | ||||||||||||||||||||||
| var errorMessage = childExitCode switch | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| ExitCodeConstants.FailedToBuildArtifacts => RunCommandStrings.AppHostFailedToBuild, | ||||||||||||||||||||||
| _ => string.Format(CultureInfo.CurrentCulture, RunCommandStrings.AppHostExitedWithCode, childExitCode) | ||||||||||||||||||||||
| }; | ||||||||||||||||||||||
| _interactionService.DisplayError(errorMessage); | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| else | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
|
|
@@ -846,11 +845,11 @@ private async Task<int> ExecuteDetachedAsync(ParseResult parseResult, FileInfo? | |||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Always show log file path for troubleshooting | ||||||||||||||||||||||
| // Point to the child's log file — it contains the actual build/runtime errors | ||||||||||||||||||||||
| _interactionService.DisplayMessage("magnifying_glass_tilted_right", string.Format( | ||||||||||||||||||||||
| CultureInfo.CurrentCulture, | ||||||||||||||||||||||
| RunCommandStrings.CheckLogsForDetails, | ||||||||||||||||||||||
| _fileLoggerProvider.LogFilePath.EscapeMarkup())); | ||||||||||||||||||||||
| childLogFile.EscapeMarkup())); | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return ExitCodeConstants.FailedToDotnetRunAppHost; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
@@ -893,4 +892,12 @@ private async Task<int> ExecuteDetachedAsync(ParseResult parseResult, FileInfo? | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| return ExitCodeConstants.Success; | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| private string GenerateChildLogFilePath() | ||||||||||||||||||||||
| { | ||||||||||||||||||||||
| return Diagnostics.FileLoggerProvider.GenerateLogFilePath( | ||||||||||||||||||||||
| ExecutionContext.LogsDirectory.FullName, | ||||||||||||||||||||||
| _timeProvider, | ||||||||||||||||||||||
| suffix: "detach-child"); | ||||||||||||||||||||||
|
Comment on lines
+898
to
+901
|
||||||||||||||||||||||
| return Diagnostics.FileLoggerProvider.GenerateLogFilePath( | |
| ExecutionContext.LogsDirectory.FullName, | |
| _timeProvider, | |
| suffix: "detach-child"); | |
| var logsDirectory = ExecutionContext.LogsDirectory.FullName; | |
| var now = _timeProvider.GetUtcNow(); | |
| var timestamp = now.ToString("yyyyMMddHHmmssfff", CultureInfo.InvariantCulture); | |
| var uniqueId = Guid.NewGuid().ToString("N", CultureInfo.InvariantCulture); | |
| var fileName = $"aspire-detach-child-{timestamp}-{uniqueId}.log"; | |
| return Path.Combine(logsDirectory, fileName); |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes to detached execution introduced new behaviors that are currently untested (e.g.,
--log-filepropagation, exit-code-to-message mapping forFailedToBuildArtifacts, and ensuring the correct log file path is surfaced on failure). There are existingRunCommandTests, but none cover--detach; adding focused unit/integration tests for these paths would help prevent regressions on Windows and in programmatic consumers.