From ef2615a293127d706b8cb3609ae573bd6aeb6be3 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 20 May 2026 19:40:58 +0800 Subject: [PATCH 1/3] Move error handling from Program.cs to BaseCommand SetAction delegate Move the OperationCanceledException and generic Exception catch blocks from Program.Main into the BaseCommand try/catch around ExecuteAsync. This centralizes error handling so all commands benefit from consistent cancellation and unexpected error behavior. Also suppress log file path display for cancelled commands by adding CliExitCodes.Cancelled to the suppression list. --- src/Aspire.Cli/Commands/BaseCommand.cs | 14 +++++++++++++- src/Aspire.Cli/Program.cs | 12 ------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/Aspire.Cli/Commands/BaseCommand.cs b/src/Aspire.Cli/Commands/BaseCommand.cs index 5e1301c1684..f6f74727b16 100644 --- a/src/Aspire.Cli/Commands/BaseCommand.cs +++ b/src/Aspire.Cli/Commands/BaseCommand.cs @@ -15,6 +15,8 @@ namespace Aspire.Cli.Commands; internal abstract class BaseCommand : Command { + private static readonly int[] s_suppressErrorLogsMessageExitCodes = [CliExitCodes.Cancelled, CliExitCodes.MissingRequiredArgument]; + protected virtual bool UpdateNotificationsEnabled { get; } = true; /// @@ -60,6 +62,16 @@ protected BaseCommand(string name, string description, IFeatures features, ICliU // Error messages have already been displayed by the interaction service. result = CommandResult.Failure(CliExitCodes.MissingRequiredArgument); } + catch (OperationCanceledException ex) when (cancellationToken.IsCancellationRequested || ex is ExtensionOperationCanceledException) + { + result = CommandResult.Cancelled(); + } + catch (Exception ex) + { + var errorMessage = string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.UnexpectedErrorOccurred, ex.Message); + telemetry.RecordError(errorMessage, ex); + result = CommandResult.Failure(CliExitCodes.InvalidCommand, errorMessage); + } var isErrorExitCode = result.ExitCode != CliExitCodes.Success; @@ -82,7 +94,7 @@ protected BaseCommand(string name, string description, IFeatures features, ICliU // Display the CLI log file path on non-zero exit codes so the user knows // where to find diagnostic details. Suppress for user-input errors where // the log wouldn't contain useful context (e.g., missing required arguments). - if (isErrorExitCode && result.ExitCode != CliExitCodes.MissingRequiredArgument) + if (isErrorExitCode && !s_suppressErrorLogsMessageExitCodes.Contains(result.ExitCode)) { interactionService.DisplayMessage( KnownEmojis.PageFacingUp, diff --git a/src/Aspire.Cli/Program.cs b/src/Aspire.Cli/Program.cs index be8e2e9934b..58c318e5a30 100644 --- a/src/Aspire.Cli/Program.cs +++ b/src/Aspire.Cli/Program.cs @@ -865,18 +865,6 @@ public static async Task Main(string[] args) // Log exit code for debugging logger.LogInformation("Exit code: {ExitCode}", exitCode); } - catch (OperationCanceledException ex) when (cancellationManager.IsCancellationRequested || ex is ExtensionOperationCanceledException) - { - exitCode = CliExitCodes.Cancelled; - } - catch (Exception ex) - { - exitCode = CliExitCodes.InvalidCommand; - - logger.LogError(ex, "An unexpected error occurred."); - telemetry.RecordError("An unexpected error occurred.", ex); - errorWriter.WriteLine(string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.UnexpectedErrorOccurred, ex.Message)); - } finally { mainActivity?.SetTag(TelemetryConstants.Tags.ProcessExitCode, exitCode); From e3f956cc2cf90cd2a12bdee0978c7dcb1adc1bf9 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 20 May 2026 19:49:48 +0800 Subject: [PATCH 2/3] Add test for unexpected exception handling in BaseCommand --- .../Commands/BaseCommandTests.cs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tests/Aspire.Cli.Tests/Commands/BaseCommandTests.cs b/tests/Aspire.Cli.Tests/Commands/BaseCommandTests.cs index 6124e5afd9d..96a214fabe2 100644 --- a/tests/Aspire.Cli.Tests/Commands/BaseCommandTests.cs +++ b/tests/Aspire.Cli.Tests/Commands/BaseCommandTests.cs @@ -270,4 +270,39 @@ public async Task BaseCommand_OnSuccess_DoesNotDisplayLogFilePath() Assert.DoesNotContain(testInteractionService.DisplayedMessages, m => m.ConsoleOverride == ConsoleOutput.Error); } + + [Fact] + public async Task BaseCommand_OnUnexpectedException_ReturnsInvalidCommandExitCode_AndDisplaysError() + { + using var workspace = TemporaryWorkspace.Create(outputHelper); + var testInteractionService = new TestInteractionService(); + var backchannelMonitor = new TestAuxiliaryBackchannelMonitor + { + ScanAsyncCallback = _ => throw new InvalidOperationException("Something went wrong") + }; + + var services = CliTestHelper.CreateServiceCollection(workspace, outputHelper, options => + { + options.InteractionServiceFactory = _ => testInteractionService; + options.AuxiliaryBackchannelMonitorFactory = _ => backchannelMonitor; + }); + using var provider = services.BuildServiceProvider(); + + var command = provider.GetRequiredService(); + var result = command.Parse("ps"); + + var exitCode = await result.InvokeAsync().DefaultTimeout(); + + Assert.Equal(CliExitCodes.InvalidCommand, exitCode); + + // Verify error message was displayed + var expectedErrorMessage = string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.UnexpectedErrorOccurred, "Something went wrong"); + Assert.Contains(expectedErrorMessage, testInteractionService.DisplayedErrors); + + // Verify log file path was displayed on stderr + var executionContext = provider.GetRequiredService(); + var expectedLogMessage = string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.SeeLogsAt, executionContext.LogFilePath); + var logMessage = Assert.Single(testInteractionService.DisplayedMessages, m => m.Message == expectedLogMessage); + Assert.Equal(ConsoleOutput.Error, logMessage.ConsoleOverride); + } } From ba7be86c7eb12e291972d61d465c6e71a2d7b35b Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 20 May 2026 21:21:57 +0800 Subject: [PATCH 3/3] Add defensive catch-all in Program.Main as safety net --- src/Aspire.Cli/Program.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Aspire.Cli/Program.cs b/src/Aspire.Cli/Program.cs index 58c318e5a30..d18df618aa5 100644 --- a/src/Aspire.Cli/Program.cs +++ b/src/Aspire.Cli/Program.cs @@ -865,6 +865,15 @@ public static async Task Main(string[] args) // Log exit code for debugging logger.LogInformation("Exit code: {ExitCode}", exitCode); } + catch (Exception ex) + { + // Should never get here because RootCommand's handler should catch all exceptions, but log just in case. + exitCode = CliExitCodes.InvalidCommand; + + logger.LogError(ex, "An unexpected error occurred."); + telemetry.RecordError("An unexpected error occurred.", ex); + errorWriter.WriteLine(string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.UnexpectedErrorOccurred, ex.Message)); + } finally { mainActivity?.SetTag(TelemetryConstants.Tags.ProcessExitCode, exitCode);