diff --git a/cli/azd/cmd/middleware/error.go b/cli/azd/cmd/middleware/error.go index 35672d640dd..f0c53bfc82d 100644 --- a/cli/azd/cmd/middleware/error.go +++ b/cli/azd/cmd/middleware/error.go @@ -157,7 +157,8 @@ func shouldSkipErrorAnalysis(err error) bool { errors.Is(err, azdcontext.ErrNoProject) || errors.Is(err, consent.ErrToolExecutionDenied) || errors.Is(err, consent.ErrElicitationDenied) || - errors.Is(err, consent.ErrSamplingDenied) { + errors.Is(err, consent.ErrSamplingDenied) || + errors.Is(err, internal.ErrAbortedByUser) { return true } diff --git a/cli/azd/cmd/middleware/error_test.go b/cli/azd/cmd/middleware/error_test.go index f4ab307f0d4..afe6e917f31 100644 --- a/cli/azd/cmd/middleware/error_test.go +++ b/cli/azd/cmd/middleware/error_test.go @@ -369,6 +369,15 @@ func Test_ShouldSkipErrorAnalysis(t *testing.T) { wrapped := fmt.Errorf("prompt failed: %w", surveyterm.InterruptErr) require.True(t, shouldSkipErrorAnalysis(wrapped)) }) + + t.Run("ErrAbortedByUser is skipped", func(t *testing.T) { + require.True(t, shouldSkipErrorAnalysis(internal.ErrAbortedByUser)) + }) + + t.Run("Wrapped ErrAbortedByUser is skipped", func(t *testing.T) { + wrapped := fmt.Errorf("preflight declined: %w", internal.ErrAbortedByUser) + require.True(t, shouldSkipErrorAnalysis(wrapped)) + }) } func Test_TroubleshootCategory_Constants(t *testing.T) { diff --git a/cli/azd/cmd/middleware/ux.go b/cli/azd/cmd/middleware/ux.go index 4539f881936..3685ca2a7da 100644 --- a/cli/azd/cmd/middleware/ux.go +++ b/cli/azd/cmd/middleware/ux.go @@ -44,6 +44,12 @@ func (m *UxMiddleware) Run(ctx context.Context, next NextFn) (*actions.ActionRes // Stop the spinner always to un-hide cursor m.console.StopSpinner(ctx, "", input.Step) + // User intentionally aborted — not a failure. + // The action already printed a message; swallow the error so the CLI exits with code 0. + if errors.Is(err, internal.ErrAbortedByUser) { + return actionResult, nil + } + if err != nil { // Use ErrorWithSuggestion for errors with suggestions (better UX). // This catches errors wrapped by the error pipeline's YAML rules diff --git a/cli/azd/cmd/middleware/ux_test.go b/cli/azd/cmd/middleware/ux_test.go new file mode 100644 index 00000000000..ba37b777c21 --- /dev/null +++ b/cli/azd/cmd/middleware/ux_test.go @@ -0,0 +1,78 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package middleware + +import ( + "context" + "errors" + "testing" + + "github.com/azure/azure-dev/cli/azd/cmd/actions" + "github.com/azure/azure-dev/cli/azd/internal" + "github.com/azure/azure-dev/cli/azd/pkg/alpha" + "github.com/azure/azure-dev/cli/azd/test/mocks" + "github.com/stretchr/testify/require" +) + +func TestUxMiddleware_ErrAbortedByUser_SwallowsError(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + featureManager := &alpha.FeatureManager{} + ux := NewUxMiddleware(&Options{}, mockContext.Console, featureManager) + + result, err := ux.Run(*mockContext.Context, func(ctx context.Context) (*actions.ActionResult, error) { + return nil, internal.ErrAbortedByUser + }) + + // Error should be swallowed (exit code 0) + require.NoError(t, err) + require.Nil(t, result) +} + +func TestUxMiddleware_ErrAbortedByUser_ChildAction_PassesThrough(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + childCtx := WithChildAction(*mockContext.Context) + featureManager := &alpha.FeatureManager{} + ux := NewUxMiddleware(&Options{}, mockContext.Console, featureManager) + + result, err := ux.Run(childCtx, func(ctx context.Context) (*actions.ActionResult, error) { + return nil, internal.ErrAbortedByUser + }) + + // For child actions, error should pass through unchanged + require.ErrorIs(t, err, internal.ErrAbortedByUser) + require.Nil(t, result) +} + +func TestUxMiddleware_OtherErrors_NotSwallowed(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + featureManager := &alpha.FeatureManager{} + ux := NewUxMiddleware(&Options{}, mockContext.Console, featureManager) + someErr := errors.New("deployment failed") + + _, err := ux.Run(*mockContext.Context, func(ctx context.Context) (*actions.ActionResult, error) { + return nil, someErr + }) + + // Other errors should still be returned + require.ErrorIs(t, err, someErr) +} + +func TestUxMiddleware_Success_ShowsActionResult(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + featureManager := &alpha.FeatureManager{} + ux := NewUxMiddleware(&Options{}, mockContext.Console, featureManager) + + actionResult := &actions.ActionResult{ + Message: &actions.ResultMessage{ + Header: "All done!", + }, + } + + result, err := ux.Run(*mockContext.Context, func(ctx context.Context) (*actions.ActionResult, error) { + return actionResult, nil + }) + + require.NoError(t, err) + require.Equal(t, actionResult, result) +} diff --git a/cli/azd/internal/cmd/errors.go b/cli/azd/internal/cmd/errors.go index 74f26ab8ae6..32fc26169c2 100644 --- a/cli/azd/internal/cmd/errors.go +++ b/cli/azd/internal/cmd/errors.go @@ -271,6 +271,8 @@ func classifySentinel(err error) string { return "internal.resource_not_found" case errors.Is(err, internal.ErrOperationCancelled): return "internal.operation_cancelled" + case errors.Is(err, internal.ErrAbortedByUser): + return "internal.operation_aborted" case errors.Is(err, terminal.InterruptErr), errors.Is(err, context.Canceled): return "user.canceled" diff --git a/cli/azd/internal/cmd/errors_test.go b/cli/azd/internal/cmd/errors_test.go index 843e5948e94..fae2f836fe7 100644 --- a/cli/azd/internal/cmd/errors_test.go +++ b/cli/azd/internal/cmd/errors_test.go @@ -1018,6 +1018,7 @@ func Test_ClassifySuggestionType_MatchesMapError(t *testing.T) { {name: "ErrKeyNotFound", err: internal.ErrKeyNotFound}, {name: "ErrExtensionNotFound", err: internal.ErrExtensionNotFound}, {name: "ErrOperationCancelled", err: internal.ErrOperationCancelled}, + {name: "ErrAbortedByUser", err: internal.ErrAbortedByUser}, // Network error { name: "DNSError", diff --git a/cli/azd/internal/cmd/provision.go b/cli/azd/internal/cmd/provision.go index cc07433ba80..03ef4c2c1e7 100644 --- a/cli/azd/internal/cmd/provision.go +++ b/cli/azd/internal/cmd/provision.go @@ -285,7 +285,7 @@ func (p *ProvisionAction) Run(ctx context.Context) (*actions.ActionResult, error return nil, fmt.Errorf("initializing provisioning manager: %w", err) } - if i == 0 { // only display once + if i == 0 && p.subManager != nil { // only display once // Get Subscription to Display in Command Title Note // Subscription and Location are ONLY displayed when they are available (found from env), otherwise, this message // is not displayed. @@ -431,6 +431,13 @@ func (p *ProvisionAction) Run(ctx context.Context) (*actions.ActionResult, error continue } + if deployResult.SkippedReason == provisioning.PreflightAbortedSkipped { + p.console.MessageUxItem(ctx, &ux.ActionResult{ + SuccessMessage: "Provisioning was cancelled.", + }) + return nil, internal.ErrAbortedByUser + } + servicesStable, err := p.importManager.ServiceStable(ctx, p.projectConfig) if err != nil { return nil, err diff --git a/cli/azd/internal/cmd/provision_test.go b/cli/azd/internal/cmd/provision_test.go new file mode 100644 index 00000000000..c1db8ae5333 --- /dev/null +++ b/cli/azd/internal/cmd/provision_test.go @@ -0,0 +1,186 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package cmd + +import ( + "context" + "io" + "os" + "path/filepath" + "testing" + + "github.com/azure/azure-dev/cli/azd/internal" + "github.com/azure/azure-dev/cli/azd/pkg/alpha" + "github.com/azure/azure-dev/cli/azd/pkg/cloud" + "github.com/azure/azure-dev/cli/azd/pkg/config" + "github.com/azure/azure-dev/cli/azd/pkg/environment" + "github.com/azure/azure-dev/cli/azd/pkg/ext" + "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning" + "github.com/azure/azure-dev/cli/azd/pkg/ioc" + "github.com/azure/azure-dev/cli/azd/pkg/output" + "github.com/azure/azure-dev/cli/azd/pkg/project" + "github.com/azure/azure-dev/cli/azd/test/mocks" + "github.com/azure/azure-dev/cli/azd/test/mocks/mockinput" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +// mockProjectManager implements project.ProjectManager for testing. +type mockProjectManager struct { + mock.Mock +} + +func (m *mockProjectManager) Initialize(ctx context.Context, projectConfig *project.ProjectConfig) error { + return m.Called(ctx, projectConfig).Error(0) +} + +func (m *mockProjectManager) EnsureAllTools( + ctx context.Context, projectConfig *project.ProjectConfig, _ project.ServiceFilterPredicate, +) error { + return m.Called(ctx, projectConfig).Error(0) +} + +func (m *mockProjectManager) DefaultServiceFromWd( + ctx context.Context, projectConfig *project.ProjectConfig, +) (*project.ServiceConfig, error) { + args := m.Called(ctx, projectConfig) + return args.Get(0).(*project.ServiceConfig), args.Error(1) +} + +func (m *mockProjectManager) EnsureFrameworkTools( + ctx context.Context, projectConfig *project.ProjectConfig, _ project.ServiceFilterPredicate, +) error { + return m.Called(ctx, projectConfig).Error(0) +} + +func (m *mockProjectManager) EnsureServiceTargetTools( + ctx context.Context, projectConfig *project.ProjectConfig, _ project.ServiceFilterPredicate, +) error { + return m.Called(ctx, projectConfig).Error(0) +} + +func (m *mockProjectManager) EnsureRestoreTools( + ctx context.Context, projectConfig *project.ProjectConfig, _ project.ServiceFilterPredicate, +) error { + return m.Called(ctx, projectConfig).Error(0) +} + +// mockProvider implements provisioning.Provider for testing. +type mockProvider struct { + deployResult *provisioning.DeployResult + deployErr error +} + +func (p *mockProvider) Name() string { return "test" } + +func (p *mockProvider) Initialize(_ context.Context, _ string, _ provisioning.Options) error { + return nil +} + +func (p *mockProvider) State(_ context.Context, _ *provisioning.StateOptions) (*provisioning.StateResult, error) { + return nil, nil +} + +func (p *mockProvider) Deploy(_ context.Context) (*provisioning.DeployResult, error) { + return p.deployResult, p.deployErr +} + +func (p *mockProvider) Preview(_ context.Context) (*provisioning.DeployPreviewResult, error) { + return nil, nil +} + +func (p *mockProvider) Destroy(_ context.Context, _ provisioning.DestroyOptions) (*provisioning.DestroyResult, error) { + return nil, nil +} + +func (p *mockProvider) EnsureEnv(_ context.Context) error { return nil } + +func (p *mockProvider) Parameters(_ context.Context) ([]provisioning.Parameter, error) { + return nil, nil +} + +// TestProvisionAction_PreflightAborted verifies that when the user declines +// preflight warnings, ProvisionAction.Run returns ErrAbortedByUser and does NOT +// attempt to read deployResult.Deployment.Outputs (which would nil-panic). +// +// Regression test for https://github.com/Azure/azure-dev/issues/7305 +func TestProvisionAction_PreflightAborted(t *testing.T) { + // Set up a temp project with a minimal infra directory so ImportManager works. + projectDir := t.TempDir() + infraDir := filepath.Join(projectDir, "infra") + require.NoError(t, os.MkdirAll(infraDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(infraDir, "main.bicep"), []byte("targetScope = 'subscription'\n"), 0o600)) + + // Mock provider that simulates preflight abort (user said No). + provider := &mockProvider{ + deployResult: &provisioning.DeployResult{ + SkippedReason: provisioning.PreflightAbortedSkipped, + }, + } + + // Register mock provider in IoC so provisioning.Manager.Initialize can resolve it. + container := ioc.NewNestedContainer(nil) + ioc.RegisterNamedInstance[provisioning.Provider](container, string(provisioning.Test), provider) + + env := environment.New("test-env") + env.SetSubscriptionId("00000000-0000-0000-0000-000000000000") + env.SetLocation("eastus2") + + console := mockinput.NewMockConsole() + + provisionManager := provisioning.NewManager( + container, + func() (provisioning.ProviderKind, error) { return provisioning.Test, nil }, + nil, // envManager — not needed for this test path + env, + console, + alpha.NewFeaturesManagerWithConfig(config.NewEmptyConfig()), + nil, // fileShareService + cloud.AzurePublic(), + ) + + pm := &mockProjectManager{} + pm.On("Initialize", mock.Anything, mock.Anything).Return(nil) + pm.On("EnsureAllTools", mock.Anything, mock.Anything).Return(nil) + + projectConfig := &project.ProjectConfig{ + Name: "test-project", + Path: projectDir, + Infra: provisioning.Options{ + Provider: provisioning.Test, + Path: "infra", + Module: "main", + }, + } + projectConfig.EventDispatcher = ext.NewEventDispatcher[project.ProjectLifecycleEventArgs]( + project.ProjectEvents..., + ) + + action := &ProvisionAction{ + flags: &ProvisionFlags{ + global: &internal.GlobalCommandOptions{}, + EnvFlag: &internal.EnvFlag{}, + }, + provisionManager: provisionManager, + projectManager: pm, + importManager: project.NewImportManager(nil), + projectConfig: projectConfig, + env: env, + console: console, + formatter: &output.NoneFormatter{}, + writer: io.Discard, + alphaFeatureManager: alpha.NewFeaturesManagerWithConfig(config.NewEmptyConfig()), + portalUrlBase: "https://portal.azure.com", + } + + mockContext := mocks.NewMockContext(context.Background()) + result, err := action.Run(*mockContext.Context) + + // Must return ErrAbortedByUser (not nil, not a panic) + require.ErrorIs(t, err, internal.ErrAbortedByUser) + require.Nil(t, result) + + // Verify project manager was called (action didn't exit prematurely) + pm.AssertExpectations(t) +} diff --git a/cli/azd/internal/errors.go b/cli/azd/internal/errors.go index 32ade664236..0a75d42fc51 100644 --- a/cli/azd/internal/errors.go +++ b/cli/azd/internal/errors.go @@ -78,6 +78,10 @@ var ( ErrNoArgsProvided = errors.New("required arguments not provided") ErrInvalidArgValue = errors.New("invalid argument value") ErrOperationCancelled = errors.New("operation cancelled by user") + + // ErrAbortedByUser indicates the user intentionally declined to proceed (e.g. preflight warnings). + // This is not a failure — the CLI should exit with code 0. + ErrAbortedByUser = errors.New("operation aborted by user") ) // Config errors diff --git a/cli/azd/pkg/workflow/runner.go b/cli/azd/pkg/workflow/runner.go index 566302bd17e..5f353a43eba 100644 --- a/cli/azd/pkg/workflow/runner.go +++ b/cli/azd/pkg/workflow/runner.go @@ -5,9 +5,11 @@ package workflow import ( "context" + "errors" "fmt" "strings" + "github.com/azure/azure-dev/cli/azd/internal" "github.com/azure/azure-dev/cli/azd/pkg/input" ) @@ -44,6 +46,11 @@ func (r *Runner) Run(ctx context.Context, workflow *Workflow) error { cancel() if err != nil { + // User intentionally aborted — stop the workflow without wrapping the error. + // Returning the original error preserves errors.Is checks upstream. + if errors.Is(err, internal.ErrAbortedByUser) { + return err + } return fmt.Errorf("error executing step command '%s': %w", strings.Join(step.AzdCommand.Args, " "), err) } } diff --git a/cli/azd/pkg/workflow/runner_test.go b/cli/azd/pkg/workflow/runner_test.go new file mode 100644 index 00000000000..aa16276e03f --- /dev/null +++ b/cli/azd/pkg/workflow/runner_test.go @@ -0,0 +1,151 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package workflow + +import ( + "context" + "errors" + "fmt" + "testing" + + "github.com/azure/azure-dev/cli/azd/internal" + "github.com/azure/azure-dev/cli/azd/test/mocks" + "github.com/stretchr/testify/require" +) + +type mockCommandRunner struct { + execFn func(ctx context.Context, args []string) error +} + +func (m *mockCommandRunner) ExecuteContext(ctx context.Context, args []string) error { + return m.execFn(ctx, args) +} + +func TestRunner_Run_StopsOnErrAbortedByUser(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + + stepsCalled := []string{} + + runner := NewRunner(&mockCommandRunner{ + execFn: func(ctx context.Context, args []string) error { + stepsCalled = append(stepsCalled, args[0]) + if args[0] == "provision" { + return internal.ErrAbortedByUser + } + return nil + }, + }, mockContext.Console) + + workflow := &Workflow{ + Name: "up", + Steps: []*Step{ + {AzdCommand: Command{Args: []string{"package", "--all"}}}, + {AzdCommand: Command{Args: []string{"provision"}}}, + {AzdCommand: Command{Args: []string{"deploy", "--all"}}}, + }, + } + + err := runner.Run(*mockContext.Context, workflow) + + // ErrAbortedByUser should propagate without wrapping + require.ErrorIs(t, err, internal.ErrAbortedByUser) + // "deploy" should NOT have been called + require.Equal(t, []string{"package", "provision"}, stepsCalled) +} + +func TestRunner_Run_ErrAbortedByUser_NotWrapped(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + + runner := NewRunner(&mockCommandRunner{ + execFn: func(ctx context.Context, args []string) error { + return internal.ErrAbortedByUser + }, + }, mockContext.Console) + + workflow := &Workflow{ + Name: "test", + Steps: []*Step{ + {AzdCommand: Command{Args: []string{"provision"}}}, + }, + } + + err := runner.Run(*mockContext.Context, workflow) + + // The error should be exactly ErrAbortedByUser, not wrapped + require.ErrorIs(t, err, internal.ErrAbortedByUser) + require.Equal(t, internal.ErrAbortedByUser.Error(), err.Error()) +} + +func TestRunner_Run_OtherErrors_AreWrapped(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + originalErr := errors.New("some deployment error") + + runner := NewRunner(&mockCommandRunner{ + execFn: func(ctx context.Context, args []string) error { + return originalErr + }, + }, mockContext.Console) + + workflow := &Workflow{ + Name: "test", + Steps: []*Step{ + {AzdCommand: Command{Args: []string{"provision"}}}, + }, + } + + err := runner.Run(*mockContext.Context, workflow) + + // Other errors should be wrapped with step context + require.ErrorIs(t, err, originalErr) + require.Contains(t, err.Error(), "error executing step command 'provision'") +} + +func TestRunner_Run_AllStepsSucceed(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + stepsCalled := []string{} + + runner := NewRunner(&mockCommandRunner{ + execFn: func(ctx context.Context, args []string) error { + stepsCalled = append(stepsCalled, args[0]) + return nil + }, + }, mockContext.Console) + + workflow := &Workflow{ + Name: "up", + Steps: []*Step{ + {AzdCommand: Command{Args: []string{"package"}}}, + {AzdCommand: Command{Args: []string{"provision"}}}, + {AzdCommand: Command{Args: []string{"deploy"}}}, + }, + } + + err := runner.Run(*mockContext.Context, workflow) + + require.NoError(t, err) + require.Equal(t, []string{"package", "provision", "deploy"}, stepsCalled) +} + +func TestRunner_Run_WrappedErrAbortedByUser(t *testing.T) { + mockContext := mocks.NewMockContext(context.Background()) + + runner := NewRunner(&mockCommandRunner{ + execFn: func(ctx context.Context, args []string) error { + return fmt.Errorf("inner context: %w", internal.ErrAbortedByUser) + }, + }, mockContext.Console) + + workflow := &Workflow{ + Name: "test", + Steps: []*Step{ + {AzdCommand: Command{Args: []string{"provision"}}}, + }, + } + + err := runner.Run(*mockContext.Context, workflow) + + // Even when wrapped, errors.Is should detect it and the runner should not add more wrapping + require.ErrorIs(t, err, internal.ErrAbortedByUser) + require.NotContains(t, err.Error(), "error executing step command") +}