Skip to content

Commit f98890f

Browse files
committed
Improve detach mode: fix pipe handle inheritance and unify log naming
1 parent 0e2fce7 commit f98890f

19 files changed

+160
-40
lines changed

src/Aspire.Cli/Commands/CacheCommand.cs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ protected override Task<int> ExecuteAsync(ParseResult parseResult, CancellationT
4545
{
4646
var cacheDirectory = ExecutionContext.CacheDirectory;
4747
var filesDeleted = 0;
48-
48+
4949
// Delete cache files and subdirectories
5050
if (cacheDirectory.Exists)
5151
{
@@ -110,14 +110,13 @@ protected override Task<int> ExecuteAsync(ParseResult parseResult, CancellationT
110110

111111
// Also clear the logs directory (skip current process's log file)
112112
var logsDirectory = ExecutionContext.LogsDirectory;
113-
// Log files are named cli-{timestamp}-{pid}.log, so we need to check the suffix
114-
var currentLogFileSuffix = $"-{Environment.ProcessId}.log";
113+
var currentLogFilePath = ExecutionContext.LogFilePath;
115114
if (logsDirectory.Exists)
116115
{
117116
foreach (var file in logsDirectory.GetFiles("*", SearchOption.AllDirectories))
118117
{
119118
// Skip the current process's log file to avoid deleting it while in use
120-
if (file.Name.EndsWith(currentLogFileSuffix, StringComparison.OrdinalIgnoreCase))
119+
if (file.FullName.Equals(currentLogFilePath, StringComparison.OrdinalIgnoreCase))
121120
{
122121
continue;
123122
}
@@ -167,4 +166,4 @@ protected override Task<int> ExecuteAsync(ParseResult parseResult, CancellationT
167166
}
168167
}
169168
}
170-
}
169+
}

src/Aspire.Cli/Commands/RunCommand.cs

Lines changed: 39 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System.CommandLine;
55
using System.Diagnostics;
6+
using System.Diagnostics.CodeAnalysis;
67
using System.Globalization;
78
using System.Text.Json;
89
using System.Text.Json.Serialization;
@@ -82,6 +83,11 @@ internal sealed class RunCommand : BaseCommand
8283
{
8384
Description = RunCommandStrings.IsolatedArgumentDescription
8485
};
86+
private static readonly Option<string?> s_logFileOption = new("--log-file")
87+
{
88+
Description = "Path to write the log file (used internally by --detach).",
89+
Hidden = true
90+
};
8591
private readonly Option<bool>? _startDebugSessionOption;
8692

8793
public RunCommand(
@@ -121,6 +127,7 @@ public RunCommand(
121127
Options.Add(s_detachOption);
122128
Options.Add(s_formatOption);
123129
Options.Add(s_isolatedOption);
130+
Options.Add(s_logFileOption);
124131

125132
if (ExtensionHelper.IsExtensionHost(InteractionService, out _, out _))
126133
{
@@ -134,6 +141,7 @@ public RunCommand(
134141
TreatUnmatchedTokensAsErrors = false;
135142
}
136143

144+
[RequiresUnreferencedCode("Calls Microsoft.Extensions.Configuration.ConfigurationBinder.GetValue<T>(String, T)")]
137145
protected override async Task<int> ExecuteAsync(ParseResult parseResult, CancellationToken cancellationToken)
138146
{
139147
var passedAppHostProjectFile = parseResult.GetValue(s_projectOption);
@@ -476,7 +484,7 @@ internal static int RenderAppHostSummary(
476484
new Align(new Markup($"[bold green]{dashboardLabel}[/]:"), HorizontalAlignment.Right),
477485
new Markup("[dim]N/A[/]"));
478486
}
479-
grid.AddRow(Text.Empty, Text.Empty);
487+
grid.AddRow(Text.Empty, Text.Empty);
480488
}
481489

482490
// Logs row
@@ -639,18 +647,23 @@ private async Task<int> ExecuteDetachedAsync(ParseResult parseResult, FileInfo?
639647
_logger.LogDebug("Found {Count} running instance(s) for this AppHost, stopping them first", existingSockets.Length);
640648
var manager = new RunningInstanceManager(_logger, _interactionService, _timeProvider);
641649
// Stop all running instances in parallel - don't block on failures
642-
var stopTasks = existingSockets.Select(socket =>
650+
var stopTasks = existingSockets.Select(socket =>
643651
manager.StopRunningInstanceAsync(socket, cancellationToken));
644652
await Task.WhenAll(stopTasks).ConfigureAwait(false);
645653
}
646654

647655
// Build the arguments for the child CLI process
656+
// Tell the child where to write its log so we can find it on failure.
657+
var childLogFile = GenerateChildLogFilePath();
658+
648659
var args = new List<string>
649660
{
650661
"run",
651662
"--non-interactive",
652663
"--project",
653-
effectiveAppHostFile.FullName
664+
effectiveAppHostFile.FullName,
665+
"--log-file",
666+
childLogFile
654667
};
655668

656669
// Pass through global options that should be forwarded to child CLI
@@ -687,14 +700,17 @@ private async Task<int> ExecuteDetachedAsync(ParseResult parseResult, FileInfo?
687700
dotnetPath, isDotnetHost, string.Join(" ", args));
688701
_logger.LogDebug("Working directory: {WorkingDirectory}", ExecutionContext.WorkingDirectory.FullName);
689702

690-
// Redirect stdout/stderr to suppress child output - it writes to log file anyway
703+
// Don't redirect stdout/stderr - child writes to log file anyway.
704+
// Redirecting creates pipe handles that get inherited by the AppHost grandchild,
705+
// which prevents callers using synchronous process APIs (e.g. execSync) from
706+
// detecting that the CLI has exited, since the pipe stays open until the AppHost dies.
691707
var startInfo = new ProcessStartInfo
692708
{
693709
FileName = dotnetPath,
694710
UseShellExecute = false,
695711
CreateNoWindow = true,
696-
RedirectStandardOutput = true,
697-
RedirectStandardError = true,
712+
RedirectStandardOutput = false,
713+
RedirectStandardError = false,
698714
RedirectStandardInput = false,
699715
WorkingDirectory = ExecutionContext.WorkingDirectory.FullName
700716
};
@@ -727,24 +743,6 @@ private async Task<int> ExecuteDetachedAsync(ParseResult parseResult, FileInfo?
727743
return null;
728744
}
729745

730-
// Start async reading of stdout/stderr to prevent buffer blocking
731-
// Log output for debugging purposes
732-
childProcess.OutputDataReceived += (_, e) =>
733-
{
734-
if (e.Data is not null)
735-
{
736-
_logger.LogDebug("Child stdout: {Line}", e.Data);
737-
}
738-
};
739-
childProcess.ErrorDataReceived += (_, e) =>
740-
{
741-
if (e.Data is not null)
742-
{
743-
_logger.LogDebug("Child stderr: {Line}", e.Data);
744-
}
745-
};
746-
childProcess.BeginOutputReadLine();
747-
childProcess.BeginErrorReadLine();
748746
}
749747
catch (Exception ex)
750748
{
@@ -823,10 +821,13 @@ private async Task<int> ExecuteDetachedAsync(ParseResult parseResult, FileInfo?
823821

824822
if (childExitedEarly)
825823
{
826-
_interactionService.DisplayError(string.Format(
827-
CultureInfo.CurrentCulture,
828-
RunCommandStrings.AppHostExitedWithCode,
829-
childExitCode));
824+
// Show a friendly message based on well-known exit codes from the child
825+
var errorMessage = childExitCode switch
826+
{
827+
ExitCodeConstants.FailedToBuildArtifacts => RunCommandStrings.AppHostFailedToBuild,
828+
_ => string.Format(CultureInfo.CurrentCulture, RunCommandStrings.AppHostExitedWithCode, childExitCode)
829+
};
830+
_interactionService.DisplayError(errorMessage);
830831
}
831832
else
832833
{
@@ -846,11 +847,11 @@ private async Task<int> ExecuteDetachedAsync(ParseResult parseResult, FileInfo?
846847
}
847848
}
848849

849-
// Always show log file path for troubleshooting
850+
// Point to the child's log file — it contains the actual build/runtime errors
850851
_interactionService.DisplayMessage("magnifying_glass_tilted_right", string.Format(
851852
CultureInfo.CurrentCulture,
852853
RunCommandStrings.CheckLogsForDetails,
853-
_fileLoggerProvider.LogFilePath.EscapeMarkup()));
854+
childLogFile.EscapeMarkup()));
854855

855856
return ExitCodeConstants.FailedToDotnetRunAppHost;
856857
}
@@ -893,4 +894,12 @@ private async Task<int> ExecuteDetachedAsync(ParseResult parseResult, FileInfo?
893894

894895
return ExitCodeConstants.Success;
895896
}
897+
898+
private string GenerateChildLogFilePath()
899+
{
900+
return Diagnostics.FileLoggerProvider.GenerateLogFilePath(
901+
ExecutionContext.LogsDirectory.FullName,
902+
_timeProvider,
903+
suffix: "detach-child");
904+
}
896905
}

src/Aspire.Cli/Diagnostics/FileLoggerProvider.cs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,22 @@ internal sealed class FileLoggerProvider : ILoggerProvider
2929
/// </summary>
3030
public string LogFilePath => _logFilePath;
3131

32+
/// <summary>
33+
/// Generates a unique, chronologically-sortable log file name.
34+
/// </summary>
35+
/// <param name="logsDirectory">The directory where log files will be written.</param>
36+
/// <param name="timeProvider">The time provider for timestamp generation.</param>
37+
/// <param name="suffix">An optional suffix appended before the extension (e.g. "detach-child").</param>
38+
internal static string GenerateLogFilePath(string logsDirectory, TimeProvider timeProvider, string? suffix = null)
39+
{
40+
var timestamp = timeProvider.GetUtcNow().ToString("yyyyMMdd'T'HHmmss", CultureInfo.InvariantCulture);
41+
var id = Guid.NewGuid().ToString("N")[..8];
42+
var name = suffix is null
43+
? $"cli_{timestamp}_{id}.log"
44+
: $"cli_{timestamp}_{id}_{suffix}.log";
45+
return Path.Combine(logsDirectory, name);
46+
}
47+
3248
/// <summary>
3349
/// Creates a new FileLoggerProvider that writes to the specified directory.
3450
/// </summary>
@@ -37,10 +53,7 @@ internal sealed class FileLoggerProvider : ILoggerProvider
3753
/// <param name="errorConsole">Optional console for error messages. Defaults to stderr.</param>
3854
public FileLoggerProvider(string logsDirectory, TimeProvider timeProvider, IAnsiConsole? errorConsole = null)
3955
{
40-
var pid = Environment.ProcessId;
41-
var timestamp = timeProvider.GetUtcNow().ToString("yyyy-MM-dd-HH-mm-ss", CultureInfo.InvariantCulture);
42-
// Timestamp first so files sort chronologically by name
43-
_logFilePath = Path.Combine(logsDirectory, $"cli-{timestamp}-{pid}.log");
56+
_logFilePath = GenerateLogFilePath(logsDirectory, timeProvider);
4457

4558
try
4659
{

src/Aspire.Cli/Program.cs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,28 @@ private static (LogLevel? ConsoleLogLevel, bool DebugMode) ParseLoggingOptions(s
9191
return (logLevel, debugMode);
9292
}
9393

94+
/// <summary>
95+
/// Parses --log-file from raw args before the host is built.
96+
/// Used by --detach to tell the child CLI where to write its log.
97+
/// </summary>
98+
private static string? ParseLogFileOption(string[]? args)
99+
{
100+
if (args is null)
101+
{
102+
return null;
103+
}
104+
105+
for (var i = 0; i < args.Length; i++)
106+
{
107+
if (args[i] == "--log-file" && i + 1 < args.Length)
108+
{
109+
return args[i + 1];
110+
}
111+
}
112+
113+
return null;
114+
}
115+
94116
private static string GetGlobalSettingsPath()
95117
{
96118
var usersAspirePath = GetUsersAspirePath();
@@ -158,7 +180,10 @@ internal static async Task<IHost> BuildApplicationAsync(string[] args, Dictionar
158180
// Always register FileLoggerProvider to capture logs to disk
159181
// This captures complete CLI session details for diagnostics
160182
var logsDirectory = Path.Combine(GetUsersAspirePath(), "logs");
161-
var fileLoggerProvider = new FileLoggerProvider(logsDirectory, TimeProvider.System);
183+
var logFilePath = ParseLogFileOption(args);
184+
var fileLoggerProvider = logFilePath is not null
185+
? new FileLoggerProvider(logFilePath)
186+
: new FileLoggerProvider(logsDirectory, TimeProvider.System);
162187
builder.Services.AddSingleton(fileLoggerProvider); // Register for direct access to LogFilePath
163188
builder.Services.TryAddEnumerable(ServiceDescriptor.Singleton<ILoggerProvider>(fileLoggerProvider));
164189

src/Aspire.Cli/Resources/RunCommandStrings.Designer.cs

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Aspire.Cli/Resources/RunCommandStrings.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,9 @@
226226
<data name="AppHostExitedWithCode" xml:space="preserve">
227227
<value>AppHost process exited with code {0}.</value>
228228
</data>
229+
<data name="AppHostFailedToBuild" xml:space="preserve">
230+
<value>AppHost failed to build.</value>
231+
</data>
229232
<data name="TimeoutWaitingForAppHost" xml:space="preserve">
230233
<value>Timeout waiting for AppHost to start.</value>
231234
</data>

src/Aspire.Cli/Resources/xlf/RunCommandStrings.cs.xlf

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Aspire.Cli/Resources/xlf/RunCommandStrings.de.xlf

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Aspire.Cli/Resources/xlf/RunCommandStrings.es.xlf

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/Aspire.Cli/Resources/xlf/RunCommandStrings.fr.xlf

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)