From b59df65bc3f949678bf7910970b14c7a9cbd28a9 Mon Sep 17 00:00:00 2001 From: Rajesh Kamal Date: Mon, 9 Mar 2026 13:37:42 -0700 Subject: [PATCH 1/4] Eliminate opaque errors_errorString from telemetry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Looked at error telemetry for azd 1.23.7 and 1.23.8. About 15k were just 'internal.errors_errorString' and ~3.6k were 'error.suggestion' — not useful for debugging. Why this was happening: - MapError was matching ErrorWithSuggestion wrapper before looking at the real error inside, so things like auth and ARM failures got misclassified. - Most Run() methods used bare errors.New or fmt.Errorf with no typed error, so MapError had nothing to match on and dumped them in the catch-all. What this does: - Unwrap ErrorWithSuggestion first so the real error gets classified - Add 28 typed errors covering all commands - Fix all 54 bare error sites to wrap typed errors with %w - Add ErrorWithSuggestion at each site for better agent/CLI experience - Add an AST test that blocks new bare errors from being added Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/cmd/auth_login.go | 9 +- cli/azd/cmd/auth_token.go | 7 +- cli/azd/cmd/completion.go | 6 +- cli/azd/cmd/config.go | 5 +- cli/azd/cmd/env.go | 110 ++++-- cli/azd/cmd/env_config_test.go | 2 +- cli/azd/cmd/env_remove.go | 12 +- cli/azd/cmd/extension.go | 109 ++++-- cli/azd/cmd/extensions.go | 10 +- cli/azd/cmd/hooks.go | 5 +- cli/azd/cmd/init.go | 12 +- cli/azd/cmd/mcp.go | 27 +- cli/azd/cmd/monitor.go | 18 +- cli/azd/cmd/templates.go | 34 +- cli/azd/cmd/up.go | 20 +- cli/azd/cmd/update.go | 5 +- cli/azd/internal/cmd/deploy.go | 22 +- cli/azd/internal/cmd/errors.go | 113 ++++-- cli/azd/internal/cmd/errors_test.go | 538 +++++++++++++++++++++++++++- cli/azd/internal/cmd/provision.go | 25 +- cli/azd/internal/cmd/publish.go | 45 ++- cli/azd/internal/cmd/show/show.go | 8 +- cli/azd/internal/errors.go | 90 ++++- cli/azd/test/functional/env_test.go | 4 +- 24 files changed, 1051 insertions(+), 185 deletions(-) diff --git a/cli/azd/cmd/auth_login.go b/cli/azd/cmd/auth_login.go index 8bd94be26ab..04296d52e6f 100644 --- a/cli/azd/cmd/auth_login.go +++ b/cli/azd/cmd/auth_login.go @@ -279,9 +279,12 @@ func (la *loginAction) Run(ctx context.Context) (*actions.ActionResult, error) { return nil, err } if !response { - return nil, fmt.Errorf( - "'azd auth login' is disabled when the auth mode is delegated. "+ - "Use the delegated identity to authenticate instead. Current mode: %s", loginMode) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "current auth mode is '%s': %w", loginMode, internal.ErrLoginDisabledDelegatedMode), + Suggestion: "Use the delegated identity to authenticate." + + " To switch to built-in auth, confirm the prompt when re-running 'azd auth login'.", + } } if err := la.authManager.SetBuiltInAuthMode(); err != nil { return nil, fmt.Errorf("setting auth mode: %w", err) diff --git a/cli/azd/cmd/auth_token.go b/cli/azd/cmd/auth_token.go index 786d477ae71..3c3dabf0bbf 100644 --- a/cli/azd/cmd/auth_token.go +++ b/cli/azd/cmd/auth_token.go @@ -169,7 +169,12 @@ func (a *authTokenAction) Run(ctx context.Context) (*actions.ActionResult, error if a.flags.claims != "" { c, err := base64.StdEncoding.DecodeString(a.flags.claims) if err != nil { - return nil, fmt.Errorf("invalid claims '%s': expected a base64-encoded string", a.flags.claims) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "invalid claims '%s', expected base64: %w", + a.flags.claims, internal.ErrInvalidArgValue), + Suggestion: "Provide a valid base64-encoded string for the --claims flag.", + } } claims = string(c) diff --git a/cli/azd/cmd/completion.go b/cli/azd/cmd/completion.go index abea5c9a1a2..c2e580a5022 100644 --- a/cli/azd/cmd/completion.go +++ b/cli/azd/cmd/completion.go @@ -8,6 +8,7 @@ import ( "fmt" "github.com/azure/azure-dev/cli/azd/cmd/actions" + "github.com/azure/azure-dev/cli/azd/internal" "github.com/azure/azure-dev/cli/azd/internal/figspec" "github.com/azure/azure-dev/cli/azd/pkg/extensions" "github.com/azure/azure-dev/cli/azd/pkg/output" @@ -172,7 +173,10 @@ func (a *completionAction) Run(ctx context.Context) (*actions.ActionResult, erro case shellPowerShell: err = rootCmd.GenPowerShellCompletion(a.cmd.OutOrStdout()) default: - return nil, fmt.Errorf("unsupported shell: %s", a.shell) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("unsupported shell '%s': %w", a.shell, internal.ErrUnsupportedOperation), + Suggestion: "Supported shells are 'bash', 'zsh', 'fish', and 'powershell'.", + } } if err != nil { diff --git a/cli/azd/cmd/config.go b/cli/azd/cmd/config.go index 63eca4a0513..e26d222417e 100644 --- a/cli/azd/cmd/config.go +++ b/cli/azd/cmd/config.go @@ -285,7 +285,10 @@ func (a *configGetAction) Run(ctx context.Context) (*actions.ActionResult, error value, ok := azdConfig.Get(key) if !ok { - return nil, fmt.Errorf("no value stored at path '%s'", key) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("no value at path '%s': %w", key, internal.ErrConfigKeyNotFound), + Suggestion: "Run 'azd config show' to see available configuration keys.", + } } if a.formatter.Kind() == output.JsonFormat { diff --git a/cli/azd/cmd/env.go b/cli/azd/cmd/env.go index e776bbfb620..fb237a0293f 100644 --- a/cli/azd/cmd/env.go +++ b/cli/azd/cmd/env.go @@ -220,7 +220,12 @@ func (e *envSetAction) Run(ctx context.Context) (*actions.ActionResult, error) { // Handle file input if specified if e.flags.file != "" { if len(e.args) > 0 { - return nil, fmt.Errorf("cannot combine --file flag with key-value arguments") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "cannot combine --file flag with key-value arguments: %w", + internal.ErrInvalidFlagCombination), + Suggestion: "Use either '--file ' or ' ' arguments, not both.", + } } filename := e.flags.file file, err := os.Open(filename) @@ -234,8 +239,11 @@ func (e *envSetAction) Run(ctx context.Context) (*actions.ActionResult, error) { return nil, fmt.Errorf("failed to parse file %s: %w", filename, err) } } else if len(e.args) == 0 { - //nolint:lll - return nil, fmt.Errorf("no environment values provided. Use ' ', '=', or '--file '") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrNoEnvValuesProvided, + Suggestion: "Provide values as 'azd env set '," + + " 'azd env set =', or 'azd env set --file '.", + } } else if len(e.args) == 2 && !strings.Contains(e.args[0], "=") { // Handle single key-value pair format: azd env set key value key := e.args[0] @@ -254,7 +262,11 @@ func (e *envSetAction) Run(ctx context.Context) (*actions.ActionResult, error) { // No environment values to set if len(keyValues) == 0 { - return nil, fmt.Errorf("no environment values to set") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrNoEnvValuesProvided, + Suggestion: "Provide values as 'azd env set '," + + " 'azd env set =', or 'azd env set --file '.", + } } // Apply the values @@ -353,8 +365,10 @@ type envSetSecretAction struct { func (e *envSetSecretAction) Run(ctx context.Context) (*actions.ActionResult, error) { if len(e.args) < 1 { - return nil, fmt.Errorf( - "no provided. Please provide a name as argument like: 'azd env set-secret '") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrNoArgsProvided, + Suggestion: "Run 'azd env set-secret ' specifying the secret name.", + } } secretName := e.args[0] @@ -458,7 +472,10 @@ func (e *envSetSecretAction) Run(ctx context.Context) (*actions.ActionResult, er return nil, fmt.Errorf("selecting key vault option: %w", err) } if useProjectKvPrompt == 1 { // Cancel - return nil, fmt.Errorf("operation cancelled. Run 'azd provision' to provision the project Key Vault first") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrOperationCancelled, + Suggestion: "Run 'azd provision' to provision the project Key Vault first.", + } } } @@ -777,7 +794,10 @@ func (e *envSelectAction) Run(ctx context.Context) (*actions.ActionResult, error } if len(envs) == 0 { - return nil, fmt.Errorf("no environments found. You can create one with \"azd env new \"") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrNoEnvironmentsFound, + Suggestion: "Run 'azd env new ' to create an environment.", + } } // Build list of environment names @@ -801,11 +821,12 @@ func (e *envSelectAction) Run(ctx context.Context) (*actions.ActionResult, error _, err := e.envManager.Get(ctx, environmentName) if errors.Is(err, environment.ErrNotFound) { - return nil, fmt.Errorf( - `environment '%s' does not exist. You can create it with "azd env new %s"`, - environmentName, - environmentName, - ) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("environment '%s' does not exist: %w", + environmentName, environment.ErrNotFound), + Suggestion: fmt.Sprintf( + "Run 'azd env list' to see environments, or 'azd env new %s' to create it.", environmentName), + } } else if err != nil { return nil, fmt.Errorf("ensuring environment exists: %w", err) } @@ -1305,9 +1326,10 @@ func (eg *envGetValuesAction) Run(ctx context.Context) (*actions.ActionResult, e env, err := eg.envManager.Get(ctx, name) if errors.Is(err, environment.ErrNotFound) { - return nil, fmt.Errorf( - `"environment does not exist. You can create it with "azd env new"`, - ) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("environment does not exist: %w", environment.ErrNotFound), + Suggestion: "Run 'azd env new ' to create an environment.", + } } else if err != nil { return nil, fmt.Errorf("ensuring environment exists: %w", err) } @@ -1372,7 +1394,10 @@ func newEnvGetValueAction( func (eg *envGetValueAction) Run(ctx context.Context) (*actions.ActionResult, error) { if len(eg.args) < 1 { - return nil, fmt.Errorf("no key name provided") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrNoKeyNameProvided, + Suggestion: "Run 'azd env get-value ' specifying the key name.", + } } keyName := eg.args[0] @@ -1390,11 +1415,12 @@ func (eg *envGetValueAction) Run(ctx context.Context) (*actions.ActionResult, er } env, err := eg.envManager.Get(ctx, name) if errors.Is(err, environment.ErrNotFound) { - return nil, fmt.Errorf( - `environment '%s' does not exist. You can create it with "azd env new %s"`, - name, - name, - ) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("environment '%s' does not exist: %w", + name, environment.ErrNotFound), + Suggestion: fmt.Sprintf( + "Run 'azd env list' to see environments, or 'azd env new %s' to create it.", name), + } } else if err != nil { return nil, fmt.Errorf("ensuring environment exists: %w", err) } @@ -1402,7 +1428,10 @@ func (eg *envGetValueAction) Run(ctx context.Context) (*actions.ActionResult, er values := env.Dotenv() keyValue, exists := values[keyName] if !exists { - return nil, fmt.Errorf("key '%s' not found in the environment values", keyName) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("%w: '%s'", internal.ErrKeyNotFound, keyName), + Suggestion: "Run 'azd env get-values' to see available keys, or 'azd env set ' to set one.", + } } // Directly write the key value to the writer @@ -1478,11 +1507,11 @@ func (a *envConfigGetAction) Run(ctx context.Context) (*actions.ActionResult, er env, err := a.envManager.Get(ctx, name) if errors.Is(err, environment.ErrNotFound) { - return nil, fmt.Errorf( - `environment '%s' does not exist. You can create it with "azd env new %s"`, - name, - name, - ) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("environment '%s' does not exist: %w", name, environment.ErrNotFound), + Suggestion: fmt.Sprintf( + "Run 'azd env list' to see environments, or 'azd env new %s' to create it.", name), + } } else if err != nil { return nil, fmt.Errorf("getting environment: %w", err) } @@ -1491,7 +1520,10 @@ func (a *envConfigGetAction) Run(ctx context.Context) (*actions.ActionResult, er value, ok := env.Config.Get(key) if !ok { - return nil, fmt.Errorf("no value stored at path '%s'", key) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("no value at path '%s': %w", key, internal.ErrConfigKeyNotFound), + Suggestion: "Check the key name and run 'azd env config get ' to retrieve a specific value.", + } } if a.formatter.Kind() == output.JsonFormat { @@ -1573,11 +1605,11 @@ func (a *envConfigSetAction) Run(ctx context.Context) (*actions.ActionResult, er env, err := a.envManager.Get(ctx, name) if errors.Is(err, environment.ErrNotFound) { - return nil, fmt.Errorf( - `environment '%s' does not exist. You can create it with "azd env new %s"`, - name, - name, - ) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("environment '%s' does not exist: %w", name, environment.ErrNotFound), + Suggestion: fmt.Sprintf( + "Run 'azd env list' to see environments, or 'azd env new %s' to create it.", name), + } } else if err != nil { return nil, fmt.Errorf("getting environment: %w", err) } @@ -1673,11 +1705,11 @@ func (a *envConfigUnsetAction) Run(ctx context.Context) (*actions.ActionResult, env, err := a.envManager.Get(ctx, name) if errors.Is(err, environment.ErrNotFound) { - return nil, fmt.Errorf( - `environment '%s' does not exist. You can create it with "azd env new %s"`, - name, - name, - ) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("environment '%s' does not exist: %w", name, environment.ErrNotFound), + Suggestion: fmt.Sprintf( + "Run 'azd env list' to see environments, or 'azd env new %s' to create it.", name), + } } else if err != nil { return nil, fmt.Errorf("getting environment: %w", err) } diff --git a/cli/azd/cmd/env_config_test.go b/cli/azd/cmd/env_config_test.go index 197e78ac1a3..b68f33ac3d0 100644 --- a/cli/azd/cmd/env_config_test.go +++ b/cli/azd/cmd/env_config_test.go @@ -115,7 +115,7 @@ func TestEnvConfigGet(t *testing.T) { }, path: "nonexistent", expectError: true, - errorContains: "no value stored at path", + errorContains: "no value at path", }, { name: "GetDeeplyNestedValue", diff --git a/cli/azd/cmd/env_remove.go b/cli/azd/cmd/env_remove.go index 9a81711d92a..05d8e67c16d 100644 --- a/cli/azd/cmd/env_remove.go +++ b/cli/azd/cmd/env_remove.go @@ -128,7 +128,10 @@ func (er *envRemoveAction) Run(ctx context.Context) (*actions.ActionResult, erro } if name == "" { - return nil, fmt.Errorf("no environment specified") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrNoArgsProvided, + Suggestion: "Run 'azd env remove ' specifying the environment.", + } } envs, err := er.envManager.List(ctx) @@ -141,7 +144,12 @@ func (er *envRemoveAction) Run(ctx context.Context) (*actions.ActionResult, erro }) if idx < 0 { - return nil, fmt.Errorf("environment '%s' does not exist", name) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "environment '%s' does not exist: %w", + name, environment.ErrNotFound), + Suggestion: "Run 'azd env list' to see available environments.", + } } env := envs[idx] diff --git a/cli/azd/cmd/extension.go b/cli/azd/cmd/extension.go index d74bf936451..061d8d35eb9 100644 --- a/cli/azd/cmd/extension.go +++ b/cli/azd/cmd/extension.go @@ -531,10 +531,16 @@ func (t *extensionShowItem) Display(writer io.Writer) error { func (a *extensionShowAction) Run(ctx context.Context) (*actions.ActionResult, error) { if len(a.args) == 0 { - return nil, fmt.Errorf("must specify an extension id") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrNoArgsProvided, + Suggestion: "Run 'azd extension show ' specifying the extension.", + } } if len(a.args) > 1 { - return nil, fmt.Errorf("cannot specify multiple extensions") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("cannot specify multiple extensions: %w", internal.ErrInvalidFlagCombination), + Suggestion: "Specify a single extension ID.", + } } extensionId := a.args[0] filterOptions := &extensions.FilterOptions{ @@ -646,11 +652,19 @@ func (a *extensionInstallAction) Run(ctx context.Context) (*actions.ActionResult extensionIds := a.args if len(extensionIds) == 0 { - return nil, fmt.Errorf("must specify an extension id") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrNoArgsProvided, + Suggestion: "Run 'azd extension install ' specifying one or more extensions.", + } } if len(extensionIds) > 1 && a.flags.version != "" { - return nil, fmt.Errorf("cannot specify --version flag when using multiple extensions") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "cannot specify --version with multiple extensions: %w", + internal.ErrInvalidFlagCombination), + Suggestion: "Install one extension at a time when using --version.", + } } azdVersion := currentAzdSemver() @@ -830,11 +844,19 @@ func newExtensionUninstallAction( func (a *extensionUninstallAction) Run(ctx context.Context) (*actions.ActionResult, error) { if len(a.args) > 0 && a.flags.all { - return nil, fmt.Errorf("cannot specify both an extension name and --all flag") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "cannot specify both an extension name and --all flag: %w", + internal.ErrInvalidFlagCombination), + Suggestion: "Use either 'azd extension uninstall ' or 'azd extension uninstall --all'.", + } } if len(a.args) == 0 && !a.flags.all { - return nil, fmt.Errorf("must specify an extension id or use --all flag") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrNoArgsProvided, + Suggestion: "Run 'azd extension uninstall ' or 'azd extension uninstall --all'.", + } } a.console.MessageUxItem(ctx, &ux.MessageTitle{ @@ -856,7 +878,10 @@ func (a *extensionUninstallAction) Run(ctx context.Context) (*actions.ActionResu } if len(extensionIds) == 0 { - return nil, fmt.Errorf("no extensions to uninstall") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrNoExtensionsAvailable, + Suggestion: "No extensions are currently installed. Run 'azd extension list' to verify.", + } } for _, extensionId := range extensionIds { @@ -932,15 +957,28 @@ func newExtensionUpgradeAction( func (a *extensionUpgradeAction) Run(ctx context.Context) (*actions.ActionResult, error) { if len(a.args) > 0 && a.flags.all { - return nil, fmt.Errorf("cannot specify both an extension name and --all flag") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "cannot specify both an extension name and --all flag: %w", + internal.ErrInvalidFlagCombination), + Suggestion: "Use either 'azd extension upgrade ' or 'azd extension upgrade --all'.", + } } if len(a.args) > 1 && a.flags.version != "" { - return nil, fmt.Errorf("cannot specify --version flag when using multiple extensions") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "cannot specify --version with multiple extensions: %w", + internal.ErrInvalidFlagCombination), + Suggestion: "Upgrade one extension at a time when using --version.", + } } if len(a.args) == 0 && !a.flags.all { - return nil, fmt.Errorf("must specify an extension id or use --all flag") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrNoArgsProvided, + Suggestion: "Run 'azd extension upgrade ' or 'azd extension upgrade --all'.", + } } a.console.MessageUxItem(ctx, &ux.MessageTitle{ @@ -964,7 +1002,10 @@ func (a *extensionUpgradeAction) Run(ctx context.Context) (*actions.ActionResult } if len(extensionIds) == 0 { - return nil, fmt.Errorf("no extensions to upgrade") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrNoExtensionsAvailable, + Suggestion: "No extensions are currently installed. Run 'azd extension list' to verify.", + } } for index, extensionId := range extensionIds { @@ -997,7 +1038,10 @@ func (a *extensionUpgradeAction) Run(ctx context.Context) (*actions.ActionResult if len(matches) == 0 { a.console.StopSpinner(ctx, stepMessage, input.StepFailed) - return nil, fmt.Errorf("extension %s not found", extensionId) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("extension '%s': %w", extensionId, internal.ErrExtensionNotFound), + Suggestion: "Run 'azd extension list' to browse available extensions.", + } } selectedExtension, err := selectDistinctExtension(ctx, a.console, extensionId, matches, a.flags.global) @@ -1181,11 +1225,12 @@ func (a *extensionSourceAddAction) Run(ctx context.Context) (*actions.ActionResu a.console.StopSpinner(ctx, spinnerMessage, input.GetStepResultFormat(err)) if err != nil { if errors.Is(err, extensions.ErrSourceTypeInvalid) { - return nil, fmt.Errorf( - "extension source type '%s' is not supported. Supported types are %s", - a.flags.kind, - ux.ListAsText([]string{"'file'", "'url'"}), - ) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "extension source type '%s' not supported: %w", + a.flags.kind, internal.ErrValidationFailed), + Suggestion: fmt.Sprintf("Supported source types are %s.", ux.ListAsText([]string{"'file'", "'url'"})), + } } return nil, fmt.Errorf("extension source validation failed: %w", err) @@ -1228,10 +1273,16 @@ func newExtensionSourceRemoveAction( func (a *extensionSourceRemoveAction) Run(ctx context.Context) (*actions.ActionResult, error) { if len(a.args) == 0 { - return nil, fmt.Errorf("must specify an extension source name") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrNoArgsProvided, + Suggestion: "Run 'azd extension source remove '.", + } } if len(a.args) > 1 { - return nil, fmt.Errorf("cannot specify multiple extension sources") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("cannot specify multiple extension sources: %w", internal.ErrInvalidFlagCombination), + Suggestion: "Remove one source at a time.", + } } a.console.MessageUxItem(ctx, &ux.MessageTitle{ Title: "Remove extension source (azd extension source remove)", @@ -1535,10 +1586,16 @@ func newExtensionSourceValidateAction( func (a *extensionSourceValidateAction) Run(ctx context.Context) (*actions.ActionResult, error) { if len(a.args) == 0 { - return nil, fmt.Errorf("must specify a source name, file path, or URL") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrNoArgsProvided, + Suggestion: "Run 'azd extension source validate '.", + } } if len(a.args) > 1 { - return nil, fmt.Errorf("cannot specify multiple sources") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("cannot specify multiple sources: %w", internal.ErrInvalidFlagCombination), + Suggestion: "Validate one source at a time.", + } } arg := a.args[0] @@ -1572,7 +1629,10 @@ func (a *extensionSourceValidateAction) Run(ctx context.Context) (*actions.Actio } if len(extensionList) == 0 { - return nil, fmt.Errorf("source contains no extensions") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("source contains no extensions: %w", internal.ErrValidationFailed), + Suggestion: "Verify the source configuration contains valid extension definitions.", + } } result := extensions.ValidateExtensions(extensionList, a.flags.strict) @@ -1586,7 +1646,10 @@ func (a *extensionSourceValidateAction) Run(ctx context.Context) (*actions.Actio } if !result.Valid { - return nil, fmt.Errorf("validation failed: one or more extensions have errors") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("one or more extensions have errors: %w", internal.ErrValidationFailed), + Suggestion: "Review the validation output above and fix the reported errors.", + } } return nil, nil diff --git a/cli/azd/cmd/extensions.go b/cli/azd/cmd/extensions.go index 43c440ae9df..ecf32f945ed 100644 --- a/cli/azd/cmd/extensions.go +++ b/cli/azd/cmd/extensions.go @@ -149,7 +149,10 @@ func newExtensionAction( func (a *extensionAction) Run(ctx context.Context) (*actions.ActionResult, error) { extensionId, has := a.cmd.Annotations["extension.id"] if !has { - return nil, fmt.Errorf("extension id not found") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrExtensionNotFound, + Suggestion: "This is an internal error — the extension annotation is missing from the command.", + } } extension, err := a.extensionManager.GetInstalled(extensions.FilterOptions{ @@ -228,7 +231,10 @@ func (a *extensionAction) Run(ctx context.Context) (*actions.ActionResult, error jwtToken, err := grpcserver.GenerateExtensionToken(extension, serverInfo) if err != nil { - return nil, fmt.Errorf("failed to generate extension token") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrExtensionTokenFailed, + Suggestion: "This is an internal error generating the JWT token for extension communication.", + } } allEnv = append(allEnv, diff --git a/cli/azd/cmd/hooks.go b/cli/azd/cmd/hooks.go index 15ef0b858f6..1328b1bbc8d 100644 --- a/cli/azd/cmd/hooks.go +++ b/cli/azd/cmd/hooks.go @@ -139,7 +139,10 @@ func (hra *hooksRunAction) Run(ctx context.Context) (*actions.ActionResult, erro if has, err := hra.importManager.HasService(ctx, hra.projectConfig, hra.flags.service); err != nil { return nil, err } else if !has { - return nil, fmt.Errorf("service name '%s' doesn't exist", hra.flags.service) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("service '%s': %w", hra.flags.service, internal.ErrServiceNotFound), + Suggestion: "Check the service name in azure.yaml or run 'azd show' to list services.", + } } } diff --git a/cli/azd/cmd/init.go b/cli/azd/cmd/init.go index 3ced42d700e..e2ea30060df 100644 --- a/cli/azd/cmd/init.go +++ b/cli/azd/cmd/init.go @@ -190,9 +190,10 @@ func (i *initAction) Run(ctx context.Context) (*actions.ActionResult, error) { i.lazyAzdCtx.SetValue(azdCtx) if i.flags.templateBranch != "" && i.flags.templatePath == "" { - return nil, - errors.New( - "using branch argument (-b or --branch) requires a template argument (--template or -t) to be specified") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrBranchRequiresTemplate, + Suggestion: "Add '--template ' when using '--branch'.", + } } // ensure that git is available @@ -254,7 +255,10 @@ func (i *initAction) Run(ctx context.Context) (*actions.ActionResult, error) { } if initTypeCount > 1 { - return nil, errors.New("only one of init modes: --template, --from-code, or --minimal should be set") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrMultipleInitModes, + Suggestion: "Choose one: 'azd init --template ', 'azd init --from-code', or 'azd init --minimal'.", + } } if initTypeSelect == initUnknown { diff --git a/cli/azd/cmd/mcp.go b/cli/azd/cmd/mcp.go index a4df542cecb..824f9576a61 100644 --- a/cli/azd/cmd/mcp.go +++ b/cli/azd/cmd/mcp.go @@ -245,7 +245,10 @@ func (a *mcpStartAction) Run(ctx context.Context) (*actions.ActionResult, error) extensionTools, err := mcpHost.AllTools(ctx) if err != nil { - return nil, fmt.Errorf("failed to load MCP host tools") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrMcpToolsLoadFailed, + Suggestion: "Check that MCP extensions are installed and configured correctly.", + } } allTools = append(allTools, extensionTools...) @@ -809,15 +812,26 @@ func (a *mcpConsentGrantAction) Run(ctx context.Context) (*actions.ActionResult, // Validate flag combinations if a.flags.tool != "" && a.flags.server == "" { - return nil, fmt.Errorf("--tool requires --server") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("--tool requires --server: %w", internal.ErrInvalidFlagCombination), + Suggestion: "Add '--server ' when using '--tool'.", + } } if a.flags.globalFlag && (a.flags.server != "" || a.flags.tool != "") { - return nil, fmt.Errorf("--global cannot be combined with --server or --tool") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "--global cannot be combined with --server or --tool: %w", + internal.ErrInvalidFlagCombination), + Suggestion: "Use '--global' alone, or use '--server' and '--tool' without '--global'.", + } } if !a.flags.globalFlag && a.flags.server == "" { - return nil, fmt.Errorf("specify either --global or --server") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("must specify --global or --server: %w", internal.ErrInvalidFlagCombination), + Suggestion: "Use '--global' for all servers, or '--server ' for a specific server.", + } } // Validate action type @@ -846,7 +860,10 @@ func (a *mcpConsentGrantAction) Run(ctx context.Context) (*actions.ActionResult, // For sampling context, tool-specific grants are not supported if operation == consent.OperationTypeSampling && a.flags.tool != "" { - return nil, fmt.Errorf("--tool is not supported for sampling rules") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("--tool is not supported for sampling rules: %w", internal.ErrInvalidFlagCombination), + Suggestion: "Remove '--tool' when granting sampling consent. Sampling rules apply per-server.", + } } // Build target diff --git a/cli/azd/cmd/monitor.go b/cli/azd/cmd/monitor.go index 83def5dd308..a1eea96ff58 100644 --- a/cli/azd/cmd/monitor.go +++ b/cli/azd/cmd/monitor.go @@ -5,7 +5,6 @@ package cmd import ( "context" - "errors" "fmt" "github.com/azure/azure-dev/cli/azd/cmd/actions" @@ -102,9 +101,10 @@ func (m *monitorAction) Run(ctx context.Context) (*actions.ActionResult, error) } if m.env.GetSubscriptionId() == "" { - return nil, errors.New( - "infrastructure has not been provisioned. Run `azd provision`", - ) + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrInfraNotProvisioned, + Suggestion: "Run 'azd provision' to set up infrastructure before monitoring.", + } } aspireDashboard := apphost.AspireDashboardUrl(ctx, m.env, m.alphaFeaturesManager) @@ -139,11 +139,17 @@ func (m *monitorAction) Run(ctx context.Context) (*actions.ActionResult, error) } if len(insightsResources) == 0 && (m.flags.monitorLive || m.flags.monitorLogs) { - return nil, fmt.Errorf("application does not contain an Application Insights resource") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("no Application Insights resource found: %w", internal.ErrResourceNotConfigured), + Suggestion: "Ensure your infrastructure includes an Application Insights component.", + } } if len(portalResources) == 0 && m.flags.monitorOverview { - return nil, fmt.Errorf("application does not contain an Application Insights dashboard") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("no Application Insights dashboard found: %w", internal.ErrResourceNotConfigured), + Suggestion: "Ensure your infrastructure includes an Application Insights dashboard.", + } } tenantId, err := m.subResolver.LookupTenant(ctx, m.env.GetSubscriptionId()) diff --git a/cli/azd/cmd/templates.go b/cli/azd/cmd/templates.go index f4085a692d1..5a728a5549e 100644 --- a/cli/azd/cmd/templates.go +++ b/cli/azd/cmd/templates.go @@ -11,6 +11,7 @@ import ( "strings" "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/input" "github.com/azure/azure-dev/cli/azd/pkg/output" "github.com/azure/azure-dev/cli/azd/pkg/output/ux" @@ -420,17 +421,15 @@ func (a *templateSourceAddAction) Run(ctx context.Context) (*actions.ActionResul for _, wellKnownSource := range templates.WellKnownSources { if wellKnownSource.Type == templates.SourceKind(strings.ToLower(a.flags.kind)) { a.console.StopSpinner(ctx, spinnerMessage, input.StepFailed) - return nil, fmt.Errorf( - "'%s' is a known key. It can't be used as type for the custom key '%s'. "+ - "For custom key, supported types are %s. "+ - "If you are trying to add the known source '%s', "+ - "run `azd template source add %s` (w/o the --type flag). ", - a.flags.kind, - key, - ux.ListAsText([]string{"'file'", "'url'", "'gh'"}), - a.flags.kind, - a.flags.kind, - ) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "'%s' is a known source type, cannot be used as custom key '%s': %w", + a.flags.kind, key, internal.ErrValidationFailed), + Suggestion: fmt.Sprintf( + "For custom keys, supported types are %s."+ + " To add the known source '%s', run 'azd template source add %s' without --type.", + ux.ListAsText([]string{"'file'", "'url'", "'gh'"}), a.flags.kind, a.flags.kind), + } } } @@ -447,11 +446,14 @@ func (a *templateSourceAddAction) Run(ctx context.Context) (*actions.ActionResul a.console.StopSpinner(ctx, spinnerMessage, input.GetStepResultFormat(err)) if err != nil { if errors.Is(err, templates.ErrSourceTypeInvalid) { - return nil, fmt.Errorf( - "template source type '%s' is not supported. Supported types are %s", - a.flags.kind, - ux.ListAsText([]string{"'file'", "'url'", "'gh'"}), - ) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "template source type '%s' not supported: %w", + a.flags.kind, internal.ErrValidationFailed), + Suggestion: fmt.Sprintf( + "Supported source types are %s.", + ux.ListAsText([]string{"'file'", "'url'", "'gh'"})), + } } return nil, fmt.Errorf("template source validation failed: %w", err) diff --git a/cli/azd/cmd/up.go b/cli/azd/cmd/up.go index fa794a6d6d7..8e6f8af922d 100644 --- a/cli/azd/cmd/up.go +++ b/cli/azd/cmd/up.go @@ -107,20 +107,24 @@ func (u *upAction) Run(ctx context.Context) (*actions.ActionResult, error) { updatedEnv := false if flagSub := u.flags.ProvisionFlags.Subscription(); flagSub != "" { if existing := u.env.GetSubscriptionId(); existing != "" && existing != flagSub { - return nil, fmt.Errorf( - "cannot change subscription for existing environment '%s' (current: %s, requested: %s). "+ - "Create a new environment with 'azd env new' instead", - u.env.Name(), existing, flagSub) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "environment '%s' (current: %s, requested: %s): %w", + u.env.Name(), existing, flagSub, internal.ErrCannotChangeSubscription), + Suggestion: "Run 'azd env new ' to create a new environment with a different subscription.", + } } u.env.SetSubscriptionId(flagSub) updatedEnv = true } if flagLoc := u.flags.ProvisionFlags.Location(); flagLoc != "" { if existing := u.env.GetLocation(); existing != "" && existing != flagLoc { - return nil, fmt.Errorf( - "cannot change location for existing environment '%s' (current: %s, requested: %s). "+ - "Create a new environment with 'azd env new' instead", - u.env.Name(), existing, flagLoc) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "environment '%s' (current: %s, requested: %s): %w", + u.env.Name(), existing, flagLoc, internal.ErrCannotChangeLocation), + Suggestion: "Run 'azd env new ' to create a new environment with a different location.", + } } u.env.SetLocation(flagLoc) updatedEnv = true diff --git a/cli/azd/cmd/update.go b/cli/azd/cmd/update.go index a45090ca75b..bacb80cf2b8 100644 --- a/cli/azd/cmd/update.go +++ b/cli/azd/cmd/update.go @@ -103,7 +103,10 @@ func newUpdateAction( func (a *updateAction) Run(ctx context.Context) (*actions.ActionResult, error) { // Non-production builds (dev and PR) should not self-update. if internal.IsNonProdVersion() { - return nil, fmt.Errorf("azd update is not supported for dev or PR builds") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("not supported for dev or PR builds: %w", internal.ErrUnsupportedOperation), + Suggestion: "Build from source or install a release build to use 'azd update'.", + } } // Auto-enable the alpha feature if not already enabled. diff --git a/cli/azd/internal/cmd/deploy.go b/cli/azd/internal/cmd/deploy.go index 4147b29d701..ed5d6283cf6 100644 --- a/cli/azd/internal/cmd/deploy.go +++ b/cli/azd/internal/cmd/deploy.go @@ -5,7 +5,6 @@ package cmd import ( "context" - "errors" "fmt" "io" "log" @@ -182,9 +181,10 @@ func (da *DeployAction) Run(ctx context.Context) (*actions.ActionResult, error) } if da.env.GetSubscriptionId() == "" { - return nil, errors.New( - "infrastructure has not been provisioned. Run `azd provision`", - ) + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrInfraNotProvisioned, + Suggestion: "Run 'azd provision' to set up infrastructure before deploying.", + } } targetServiceName, err := getTargetServiceName( @@ -201,15 +201,17 @@ func (da *DeployAction) Run(ctx context.Context) (*actions.ActionResult, error) } if da.flags.All && da.flags.fromPackage != "" { - return nil, errors.New( - "'--from-package' cannot be specified when '--all' is set. Specify a specific service by passing a ") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrFromPackageWithAll, + Suggestion: "Use 'azd deploy --from-package ' to target a specific service.", + } } if targetServiceName == "" && da.flags.fromPackage != "" { - return nil, errors.New( - //nolint:lll - "'--from-package' cannot be specified when deploying all services. Specify a specific service by passing a ", - ) + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrFromPackageNoService, + Suggestion: "Use 'azd deploy --from-package ' to target a specific service.", + } } if err := da.projectManager.Initialize(ctx, da.projectConfig); err != nil { diff --git a/cli/azd/internal/cmd/errors.go b/cli/azd/internal/cmd/errors.go index 516d3621f50..ef94f139782 100644 --- a/cli/azd/internal/cmd/errors.go +++ b/cli/azd/internal/cmd/errors.go @@ -26,6 +26,8 @@ import ( "github.com/azure/azure-dev/cli/azd/pkg/auth" "github.com/azure/azure-dev/cli/azd/pkg/azapi" "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/azure/azure-dev/cli/azd/pkg/environment" + "github.com/azure/azure-dev/cli/azd/pkg/environment/azdcontext" "github.com/azure/azure-dev/cli/azd/pkg/exec" "github.com/azure/azure-dev/cli/azd/pkg/extensions" "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning" @@ -59,15 +61,21 @@ func MapError(err error, span tracing.Span) { var loginErr *auth.ReLoginRequiredError var updateErr *update.UpdateError - if errors.As(err, &updateErr) { + // If the error is wrapped in ErrorWithSuggestion, unwrap it first so the inner error + // can be classified by its actual type (ResponseError, AzureDeploymentError, etc.) + // instead of being bucketed as the opaque "error.suggestion". + classifyErr := err + if errors.As(err, &errWithSuggestion) { + if inner := errWithSuggestion.Unwrap(); inner != nil { + classifyErr = inner + } + } + + if errors.As(classifyErr, &updateErr) { errCode = updateErr.Code - } else if errors.As(err, &loginErr) { + } else if errors.As(classifyErr, &loginErr) { errCode = "auth.login_required" - } else if errors.As(err, &errWithSuggestion) { - errCode = "error.suggestion" - errType := errorType(errWithSuggestion.Unwrap()) - span.SetAttributes(fields.ErrType.String(errType)) - } else if errors.As(err, &respErr) { + } else if errors.As(classifyErr, &respErr) { serviceName := "other" statusCode := -1 errDetails = append(errDetails, fields.ServiceErrorCode.String(respErr.ErrorCode)) @@ -88,7 +96,7 @@ func MapError(err error, span tracing.Span) { } errCode = fmt.Sprintf("service.%s.%d", serviceName, statusCode) - } else if errors.As(err, &armDeployErr) { + } else if errors.As(classifyErr, &armDeployErr) { errDetails = append(errDetails, fields.ServiceName.String("arm")) codes := []*deploymentErrorCode{} var collect func(details []*azapi.DeploymentErrorLine, frame int) @@ -122,7 +130,7 @@ func MapError(err error, span tracing.Span) { operation = "deployment" } errCode = fmt.Sprintf("service.arm.%s.failed", operation) - } else if errors.As(err, &extServiceErr) { + } else if errors.As(classifyErr, &extServiceErr) { // Handle structured service errors from extensions. // Emit whatever details are available rather than requiring all fields. serviceName := "" @@ -153,7 +161,7 @@ func MapError(err error, span tracing.Span) { default: errCode = "ext.service.unknown.failed" } - } else if errors.As(err, &extLocalErr) { + } else if errors.As(classifyErr, &extLocalErr) { domain := string(azdext.NormalizeLocalErrorCategory(extLocalErr.Category)) code := normalizeCodeSegment(extLocalErr.Code, "failed") @@ -163,9 +171,9 @@ func MapError(err error, span tracing.Span) { ) errCode = fmt.Sprintf("ext.%s.%s", domain, code) - } else if errors.As(err, &extensionRunErr) { + } else if errors.As(classifyErr, &extensionRunErr) { errCode = "ext.run.failed" - } else if errors.As(err, &toolExecErr) { + } else if errors.As(classifyErr, &toolExecErr) { toolName := "other" cmdName := cmdAsName(toolExecErr.Cmd) if cmdName != "" { @@ -177,7 +185,7 @@ func MapError(err error, span tracing.Span) { fields.ToolName.String(toolName)) errCode = fmt.Sprintf("tool.%s.failed", toolName) - } else if errors.As(err, &toolCheckErr) { + } else if errors.As(classifyErr, &toolCheckErr) { if len(toolCheckErr.ToolNames) == 1 { toolName := toolCheckErr.ToolNames[0] errCode = fmt.Sprintf("tool.%s.missing", toolName) @@ -186,7 +194,7 @@ func MapError(err error, span tracing.Span) { errCode = "tool.multiple.missing" errDetails = append(errDetails, fields.ToolName.String(strings.Join(toolCheckErr.ToolNames, ","))) } - } else if errors.As(err, &authFailedErr) { + } else if errors.As(classifyErr, &authFailedErr) { errDetails = append(errDetails, fields.ServiceName.String("aad")) if authFailedErr.Parsed != nil { codes := make([]string, 0, len(authFailedErr.Parsed.ErrorCodes)) @@ -200,32 +208,83 @@ func MapError(err error, span tracing.Span) { fields.ServiceCorrelationId.String(authFailedErr.Parsed.CorrelationId)) } errCode = "service.aad.failed" - } else if errors.Is(err, terminal.InterruptErr) { + } else if errors.Is(classifyErr, terminal.InterruptErr) { errCode = "user.canceled" - } else if errors.Is(err, context.Canceled) { + } else if errors.Is(classifyErr, context.Canceled) { errCode = "user.canceled" - } else if errors.Is(err, context.DeadlineExceeded) { + } else if errors.Is(classifyErr, context.DeadlineExceeded) { errCode = "internal.timeout" - } else if errors.Is(err, auth.ErrNoCurrentUser) { + } else if errors.Is(classifyErr, auth.ErrNoCurrentUser) { errCode = "auth.not_logged_in" - } else if errors.Is(err, consent.ErrToolExecutionDenied) { + } else if errors.Is(classifyErr, consent.ErrToolExecutionDenied) { errCode = "user.tool_denied" - } else if errors.Is(err, git.ErrNotRepository) { + } else if errors.Is(classifyErr, git.ErrNotRepository) { errCode = "internal.not_git_repo" - } else if errors.Is(err, azapi.ErrPreviewNotSupported) { + } else if errors.Is(classifyErr, azapi.ErrPreviewNotSupported) { errCode = "internal.preview_not_supported" - } else if errors.Is(err, provisioning.ErrBindMountOperationDisabled) { + } else if errors.Is(classifyErr, provisioning.ErrBindMountOperationDisabled) { errCode = "internal.bind_mount_disabled" - } else if errors.Is(err, update.ErrNeedsElevation) { + } else if errors.Is(classifyErr, update.ErrNeedsElevation) { errCode = "update.elevationRequired" - } else if errors.Is(err, pipeline.ErrRemoteHostIsNotAzDo) { + } else if errors.Is(classifyErr, pipeline.ErrRemoteHostIsNotAzDo) { errCode = "internal.remote_not_azdo" - } else if isNetworkError(err) { + } else if errors.Is(classifyErr, internal.ErrInfraNotProvisioned) { + errCode = "internal.infra_not_provisioned" + } else if errors.Is(classifyErr, internal.ErrFromPackageWithAll) || + errors.Is(classifyErr, internal.ErrFromPackageNoService) { + errCode = "internal.invalid_flag_combination" + } else if errors.Is(classifyErr, internal.ErrCannotChangeSubscription) { + errCode = "internal.cannot_change_subscription" + } else if errors.Is(classifyErr, internal.ErrCannotChangeLocation) { + errCode = "internal.cannot_change_location" + } else if errors.Is(classifyErr, internal.ErrPreviewMultipleLayers) { + errCode = "internal.preview_multiple_layers" + } else if errors.Is(classifyErr, internal.ErrNoKeyNameProvided) || + errors.Is(classifyErr, internal.ErrNoEnvValuesProvided) || + errors.Is(classifyErr, internal.ErrInvalidFlagCombination) { + errCode = "internal.invalid_args" + } else if errors.Is(classifyErr, internal.ErrKeyNotFound) { + errCode = "internal.key_not_found" + } else if errors.Is(classifyErr, internal.ErrNoEnvironmentsFound) { + errCode = "internal.no_environments_found" + } else if errors.Is(classifyErr, internal.ErrLoginDisabledDelegatedMode) { + errCode = "auth.login_disabled_delegated" + } else if errors.Is(classifyErr, internal.ErrBranchRequiresTemplate) || + errors.Is(classifyErr, internal.ErrMultipleInitModes) { + errCode = "internal.invalid_args" + } else if errors.Is(classifyErr, environment.ErrNotFound) { + errCode = "internal.env_not_found" + } else if errors.Is(classifyErr, azdcontext.ErrNoProject) { + errCode = "internal.no_project" + } else if errors.Is(classifyErr, internal.ErrNoArgsProvided) || + errors.Is(classifyErr, internal.ErrInvalidArgValue) { + errCode = "internal.invalid_args" + } else if errors.Is(classifyErr, internal.ErrConfigKeyNotFound) { + errCode = "internal.config_key_not_found" + } else if errors.Is(classifyErr, internal.ErrExtensionNotFound) { + errCode = "internal.extension_not_found" + } else if errors.Is(classifyErr, internal.ErrServiceNotFound) { + errCode = "internal.service_not_found" + } else if errors.Is(classifyErr, internal.ErrNoExtensionsAvailable) { + errCode = "internal.no_extensions_available" + } else if errors.Is(classifyErr, internal.ErrValidationFailed) { + errCode = "internal.validation_failed" + } else if errors.Is(classifyErr, internal.ErrUnsupportedOperation) { + errCode = "internal.unsupported_operation" + } else if errors.Is(classifyErr, internal.ErrExtensionTokenFailed) { + errCode = "internal.extension_error" + } else if errors.Is(classifyErr, internal.ErrMcpToolsLoadFailed) { + errCode = "internal.mcp_error" + } else if errors.Is(classifyErr, internal.ErrResourceNotConfigured) { + errCode = "internal.resource_not_found" + } else if errors.Is(classifyErr, internal.ErrOperationCancelled) { + errCode = "internal.operation_cancelled" + } else if isNetworkError(classifyErr) { errCode = "internal.network" - errType := errorType(err) + errType := errorType(classifyErr) span.SetAttributes(fields.ErrType.String(errType)) } else { - errType := errorType(err) + errType := errorType(classifyErr) span.SetAttributes(fields.ErrType.String(errType)) errCode = fmt.Sprintf("internal.%s", strings.ReplaceAll(strings.ReplaceAll(errType, ".", "_"), "*", "")) diff --git a/cli/azd/internal/cmd/errors_test.go b/cli/azd/internal/cmd/errors_test.go index a095c1bdd37..c765c2d7911 100644 --- a/cli/azd/internal/cmd/errors_test.go +++ b/cli/azd/internal/cmd/errors_test.go @@ -21,11 +21,14 @@ import ( "testing" "github.com/Azure/azure-sdk-for-go/sdk/azcore" + "github.com/azure/azure-dev/cli/azd/internal" "github.com/azure/azure-dev/cli/azd/internal/agent/consent" "github.com/azure/azure-dev/cli/azd/internal/tracing/fields" "github.com/azure/azure-dev/cli/azd/pkg/auth" "github.com/azure/azure-dev/cli/azd/pkg/azapi" "github.com/azure/azure-dev/cli/azd/pkg/azdext" + "github.com/azure/azure-dev/cli/azd/pkg/environment" + "github.com/azure/azure-dev/cli/azd/pkg/environment/azdcontext" "github.com/azure/azure-dev/cli/azd/pkg/exec" "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning" "github.com/azure/azure-dev/cli/azd/pkg/pipeline" @@ -382,7 +385,193 @@ func Test_MapError(t *testing.T) { fields.ErrorKey(fields.ErrCode.Key).String("token_expired"), }, }, + { + name: "WithSuggestionWrappingResponseError", + err: &internal.ErrorWithSuggestion{ + Err: &azcore.ResponseError{ + ErrorCode: "QuotaExceeded", + StatusCode: 429, + RawResponse: &http.Response{ + StatusCode: 429, + Request: &http.Request{ + Method: "POST", + Host: "management.azure.com", + }, + }, + }, + Suggestion: "Request a quota increase in the Azure portal.", + }, + wantErrReason: "service.arm.429", + wantErrDetails: []attribute.KeyValue{ + fields.ErrorKey(fields.ServiceName.Key).String("arm"), + fields.ErrorKey(fields.ServiceHost.Key).String("management.azure.com"), + fields.ErrorKey(fields.ServiceMethod.Key).String("POST"), + fields.ErrorKey(fields.ServiceErrorCode.Key).String("QuotaExceeded"), + fields.ErrorKey(fields.ServiceStatusCode.Key).Int(429), + }, + }, + { + name: "WithSuggestionWrappingPlainError", + err: &internal.ErrorWithSuggestion{ + Err: errors.New("something failed"), + Suggestion: "Try again later.", + }, + wantErrReason: "internal.errors_errorString", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String("*errors.errorString"), + }, + }, + // Sentinel error test cases — verify typed errors produce meaningful ResultCodes + { + name: "WithErrNoProject", + err: azdcontext.ErrNoProject, + wantErrReason: "internal.no_project", + }, + { + name: "WithErrEnvNotFound", + err: fmt.Errorf("environment 'dev' does not exist: %w", environment.ErrNotFound), + wantErrReason: "internal.env_not_found", + }, + { + name: "WithErrInfraNotProvisioned", + err: fmt.Errorf("run `azd provision`: %w", internal.ErrInfraNotProvisioned), + wantErrReason: "internal.infra_not_provisioned", + }, + { + name: "WithErrFromPackageWithAll", + err: fmt.Errorf("specify a service: %w", internal.ErrFromPackageWithAll), + wantErrReason: "internal.invalid_flag_combination", + }, + { + name: "WithErrFromPackageNoService", + err: fmt.Errorf("specify a service: %w", internal.ErrFromPackageNoService), + wantErrReason: "internal.invalid_flag_combination", + }, + { + name: "WithErrCannotChangeSubscription", + err: fmt.Errorf("env 'dev': %w", internal.ErrCannotChangeSubscription), + wantErrReason: "internal.cannot_change_subscription", + }, + { + name: "WithErrCannotChangeLocation", + err: fmt.Errorf("env 'dev': %w", internal.ErrCannotChangeLocation), + wantErrReason: "internal.cannot_change_location", + }, + { + name: "WithErrPreviewMultipleLayers", + err: fmt.Errorf("specify a layer: %w", internal.ErrPreviewMultipleLayers), + wantErrReason: "internal.preview_multiple_layers", + }, + { + name: "WithErrNoKeyNameProvided", + err: internal.ErrNoKeyNameProvided, + wantErrReason: "internal.invalid_args", + }, + { + name: "WithErrNoEnvValuesProvided", + err: fmt.Errorf("use key=value: %w", internal.ErrNoEnvValuesProvided), + wantErrReason: "internal.invalid_args", + }, + { + name: "WithErrInvalidFlagCombination", + err: fmt.Errorf("cannot combine flags: %w", internal.ErrInvalidFlagCombination), + wantErrReason: "internal.invalid_args", + }, + { + name: "WithErrKeyNotFound", + err: fmt.Errorf("%w: 'MY_KEY'", internal.ErrKeyNotFound), + wantErrReason: "internal.key_not_found", + }, + { + name: "WithErrNoEnvironmentsFound", + err: fmt.Errorf("create one with azd env new: %w", internal.ErrNoEnvironmentsFound), + wantErrReason: "internal.no_environments_found", + }, + { + name: "WithErrLoginDisabledDelegatedMode", + err: fmt.Errorf("current mode: az_delegated: %w", internal.ErrLoginDisabledDelegatedMode), + wantErrReason: "auth.login_disabled_delegated", + }, + { + name: "WithErrBranchRequiresTemplate", + err: fmt.Errorf("use --template: %w", internal.ErrBranchRequiresTemplate), + wantErrReason: "internal.invalid_args", + }, + { + name: "WithErrMultipleInitModes", + err: internal.ErrMultipleInitModes, + wantErrReason: "internal.invalid_args", + }, + // New sentinels — batch 2 (54 bare-error fixes) + { + name: "WithErrNoArgsProvided", + err: internal.ErrNoArgsProvided, + wantErrReason: "internal.invalid_args", + }, + { + name: "WithErrInvalidArgValue", + err: internal.ErrInvalidArgValue, + wantErrReason: "internal.invalid_args", + }, + { + name: "WithErrOperationCancelled", + err: internal.ErrOperationCancelled, + wantErrReason: "internal.operation_cancelled", + }, + { + name: "WithErrConfigKeyNotFound", + err: internal.ErrConfigKeyNotFound, + wantErrReason: "internal.config_key_not_found", + }, + { + name: "WithErrExtensionNotFound", + err: internal.ErrExtensionNotFound, + wantErrReason: "internal.extension_not_found", + }, + { + name: "WithErrNoExtensionsAvailable", + err: internal.ErrNoExtensionsAvailable, + wantErrReason: "internal.no_extensions_available", + }, + { + name: "WithErrExtensionTokenFailed", + err: internal.ErrExtensionTokenFailed, + wantErrReason: "internal.extension_error", + }, + { + name: "WithErrServiceNotFound", + err: internal.ErrServiceNotFound, + wantErrReason: "internal.service_not_found", + }, + { + name: "WithErrResourceNotConfigured", + err: internal.ErrResourceNotConfigured, + wantErrReason: "internal.resource_not_found", + }, + { + name: "WithErrValidationFailed", + err: internal.ErrValidationFailed, + wantErrReason: "internal.validation_failed", + }, + { + name: "WithErrUnsupportedOperation", + err: internal.ErrUnsupportedOperation, + wantErrReason: "internal.unsupported_operation", + }, + { + name: "WithErrMcpToolsLoadFailed", + err: internal.ErrMcpToolsLoadFailed, + wantErrReason: "internal.mcp_error", + }, + } + // Test cases that intentionally produce errors_errorString (the catch-all bucket). + // Any NEW test case that produces this is a signal that a typed sentinel is needed. + allowedCatchAll := map[string]bool{ + "WithNilError": true, + "WithOtherError": true, + "WithSuggestionWrappingPlainError": true, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { span := &mocktracing.Span{} @@ -390,6 +579,95 @@ func Test_MapError(t *testing.T) { require.Equal(t, tt.wantErrReason, span.Status.Description) require.ElementsMatch(t, tt.wantErrDetails, span.Attributes) + + // Enforcement: no test case should produce the opaque errors_errorString + // unless explicitly allowed. This catches regressions where new error paths + // return bare errors.New() without typed sentinels. + if !allowedCatchAll[tt.name] { + require.NotContains(t, span.Status.Description, "errors_errorString", + "test case %q produces opaque errors_errorString — use a typed sentinel error instead", tt.name) + } + }) + } +} + +// TestMapError_ErrorWithSuggestionUnwrapsInnerError verifies that ErrorWithSuggestion no longer +// masks the inner error's classification. Previously, any error wrapped in ErrorWithSuggestion +// was reported as "error.suggestion" in telemetry, losing ~3,600 hits of actionable error data. +// After the fix, the inner error is unwrapped and classified by its actual type. +func TestMapError_ErrorWithSuggestionUnwrapsInnerError(t *testing.T) { + tests := []struct { + name string + err error + wantErrCode string + wantNotCode string // the code it would have been before the fix + wantAttribute *attribute.KeyValue + }{ + { + name: "ResponseError_classified_by_inner_type", + err: &internal.ErrorWithSuggestion{ + Err: &azcore.ResponseError{ + ErrorCode: "QuotaExceeded", + StatusCode: 429, + RawResponse: &http.Response{ + StatusCode: 429, + Request: &http.Request{Method: "POST", Host: "management.azure.com"}, + }, + }, + Suggestion: "Request a quota increase.", + }, + wantErrCode: "service.arm.429", + wantNotCode: "error.suggestion", + }, + { + name: "ArmDeploymentError_classified_by_inner_type", + err: &internal.ErrorWithSuggestion{ + Err: &azapi.AzureDeploymentError{ + Operation: azapi.DeploymentOperationDeploy, + Details: &azapi.DeploymentErrorLine{Code: "Conflict"}, + }, + Suggestion: "Check for existing resources.", + }, + wantErrCode: "service.arm.deployment.failed", + wantNotCode: "error.suggestion", + }, + { + name: "ToolExitError_classified_by_inner_type", + err: &internal.ErrorWithSuggestion{ + Err: &exec.ExitError{Cmd: "docker", ExitCode: 1}, + Suggestion: "Run docker login.", + }, + wantErrCode: "tool.docker.failed", + wantNotCode: "error.suggestion", + }, + { + name: "PlainError_falls_through_to_internal", + err: &internal.ErrorWithSuggestion{ + Err: errors.New("unknown failure"), + Suggestion: "Try again.", + }, + wantErrCode: "internal.errors_errorString", + wantNotCode: "error.suggestion", + wantAttribute: func() *attribute.KeyValue { + kv := fields.ErrType.String("*errors.errorString") + return &kv + }(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + span := &mocktracing.Span{} + MapError(tt.err, span) + + require.Equal(t, tt.wantErrCode, span.Status.Description, + "should classify by inner error type, not as opaque error.suggestion") + require.NotEqual(t, tt.wantNotCode, span.Status.Description, + "should NOT produce the old opaque code") + + if tt.wantAttribute != nil { + require.Contains(t, span.Attributes, *tt.wantAttribute) + } }) } } @@ -645,7 +923,6 @@ func Test_PackageLevelErrorsMapped(t *testing.T) { "ErrDeploymentNotFound": "caught in provisioning/deployment callers before reaching telemetry", "ErrDeploymentsNotFound": "caught in infra callers before reaching telemetry", "ErrDeploymentResourcesNotFound": "caught in infra callers before reaching telemetry", - "ErrNoProject": "caught in environment/context callers before reaching telemetry", "ErrContainerNotFound": "caught in storage blob callers before reaching telemetry", "ErrPlatformNotSupported": "caught in platform config resolver before reaching telemetry", "ErrPlatformConfigNotFound": "caught in platform config resolver before reaching telemetry", @@ -669,7 +946,6 @@ func Test_PackageLevelErrorsMapped(t *testing.T) { // Environment management errors surfaced as user-facing messages with suggestions "ErrExists": "environment: user-facing with suggestion, wrapped before reaching telemetry", - "ErrNotFound": "environment: user-facing with suggestion, wrapped before reaching telemetry", "ErrNameNotSpecified": "environment: user-facing with suggestion, wrapped before reaching telemetry", "ErrDefaultEnvironmentNotFound": "environment: user-facing with suggestion, wrapped before reaching telemetry", @@ -821,3 +1097,261 @@ func isErrorConstructorCall(call *ast.CallExpr) bool { return (ident.Name == "errors" && sel.Sel.Name == "New") || (ident.Name == "fmt" && sel.Sel.Name == "Errorf") } + +// Test_RunMethodsNoBareErrors walks all action Run() methods and flags inline bare errors +// (errors.New or fmt.Errorf without %w) that would produce opaque errors_errorString in telemetry. +// These errors reach MapError via the telemetry middleware and must use typed sentinels or wrap +// an existing error with %w for proper classification. +func Test_RunMethodsNoBareErrors(t *testing.T) { + azdRoot, err := filepath.Abs(filepath.Join("..", "..")) + require.NoError(t, err) + + // knownBareErrors tracks pre-existing bare error violations in Run() methods. + // This list should be EMPTY — all bare errors should use typed sentinels. + // If a new bare error must be temporarily added, it requires justification. + knownBareErrors := map[string]bool{} + + var violations []string + var knownFound int + + err = filepath.Walk(azdRoot, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + if info.IsDir() { + base := filepath.Base(path) + if base == "vendor" || base == "extensions" || base == ".git" || base == "test" { + return filepath.SkipDir + } + return nil + } + + if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") { + return nil + } + + fset := token.NewFileSet() + file, parseErr := parser.ParseFile(fset, path, nil, 0) + if parseErr != nil { + return nil + } + + for _, decl := range file.Decls { + funcDecl, ok := decl.(*ast.FuncDecl) + if !ok { + continue + } + + if !isActionRunMethod(funcDecl) { + continue + } + + relPath, _ := filepath.Rel(azdRoot, path) + receiverName := getReceiverTypeName(funcDecl) + + // Walk the function body looking for return statements with bare errors + ast.Inspect(funcDecl.Body, func(n ast.Node) bool { + retStmt, ok := n.(*ast.ReturnStmt) + if !ok || len(retStmt.Results) < 2 { + return true + } + + // The error is the last return value + errExpr := retStmt.Results[len(retStmt.Results)-1] + + // Check for direct errors.New(...) or fmt.Errorf() without %w + if call, ok := errExpr.(*ast.CallExpr); ok { + if isBareErrorCall(call) { + pos := fset.Position(retStmt.Pos()) + key := fmt.Sprintf("%s:%d", relPath, pos.Line) + if knownBareErrors[key] { + knownFound++ + } else { + violations = append(violations, fmt.Sprintf( + " %s %s.Run() — bare %s (use a typed sentinel with %%w)", + key, receiverName, callName(call))) + } + } + } + + // Check for &ErrorWithSuggestion{Err: errors.New(...)} or similar + // where the inner Err field is a bare error + if unary, ok := errExpr.(*ast.UnaryExpr); ok { + if comp, ok := unary.X.(*ast.CompositeLit); ok { + checkCompositeLitForBareErr( + fset, comp, relPath, receiverName, knownBareErrors, &knownFound, &violations) + } + } + + return true + }) + } + + return nil + }) + require.NoError(t, err) + + if len(violations) > 0 { + t.Errorf( + "Found %d NEW bare error(s) in action Run() methods that would produce opaque telemetry.\n"+ + "Use typed sentinel errors (internal.ErrXxx) with %%w wrapping, or wrap with an existing\n"+ + "typed error from a dependency. Bare errors.New()/fmt.Errorf() without %%w produce\n"+ + "'internal.errors_errorString' in telemetry.\n\n"+ + "Violations:\n%s", + len(violations), + strings.Join(violations, "\n"), + ) + } + + // Verify allowlist isn't stale — if a known error was fixed, remove it from the list + if knownFound != len(knownBareErrors) { + t.Errorf( + "knownBareErrors allowlist has %d entries but only %d were found.\n"+ + "Remove fixed entries from the allowlist to keep it accurate.", + len(knownBareErrors), knownFound) + } +} + +// isActionRunMethod checks if a function declaration is a method named "Run" that returns +// (*actions.ActionResult, error) — the signature of azd action entry points. +func isActionRunMethod(fn *ast.FuncDecl) bool { + if fn.Name.Name != "Run" { + return false + } + // Must be a method (have a receiver) + if fn.Recv == nil || len(fn.Recv.List) == 0 { + return false + } + // Must have parameters (ctx context.Context) + if fn.Type.Params == nil || len(fn.Type.Params.List) == 0 { + return false + } + // Must return two values where the second is error + if fn.Type.Results == nil || len(fn.Type.Results.List) != 2 { + return false + } + + // Check first return type contains "ActionResult" (pointer to it) + firstResult := fn.Type.Results.List[0] + if star, ok := firstResult.Type.(*ast.StarExpr); ok { + if sel, ok := star.X.(*ast.SelectorExpr); ok { + if sel.Sel.Name != "ActionResult" { + return false + } + } else { + return false + } + } else { + return false + } + + // Check second return type is "error" + secondResult := fn.Type.Results.List[1] + if ident, ok := secondResult.Type.(*ast.Ident); ok { + if ident.Name != "error" { + return false + } + } else { + return false + } + + return true +} + +// getReceiverTypeName extracts the receiver type name from a method declaration. +func getReceiverTypeName(fn *ast.FuncDecl) string { + if fn.Recv == nil || len(fn.Recv.List) == 0 { + return "" + } + switch t := fn.Recv.List[0].Type.(type) { + case *ast.StarExpr: + if ident, ok := t.X.(*ast.Ident); ok { + return ident.Name + } + case *ast.Ident: + return t.Name + } + return "" +} + +// isBareErrorCall checks if a call expression is errors.New(...) or fmt.Errorf(...) without %w. +func isBareErrorCall(call *ast.CallExpr) bool { + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return false + } + ident, ok := sel.X.(*ast.Ident) + if !ok { + return false + } + + // errors.New(...) is always bare + if ident.Name == "errors" && sel.Sel.Name == "New" { + return true + } + + // fmt.Errorf(...) is bare only if format string has no %w + if ident.Name == "fmt" && sel.Sel.Name == "Errorf" { + return !fmtErrorfHasWrap(call) + } + + return false +} + +// fmtErrorfHasWrap checks if a fmt.Errorf call contains %w in the format string. +func fmtErrorfHasWrap(call *ast.CallExpr) bool { + if len(call.Args) == 0 { + return false + } + lit, ok := call.Args[0].(*ast.BasicLit) + if !ok { + return false + } + return strings.Contains(lit.Value, "%w") +} + +// callName returns a human-readable name for a call expression (e.g. "errors.New" or "fmt.Errorf"). +func callName(call *ast.CallExpr) string { + if sel, ok := call.Fun.(*ast.SelectorExpr); ok { + if ident, ok := sel.X.(*ast.Ident); ok { + return ident.Name + "." + sel.Sel.Name + } + } + return "" +} + +// checkCompositeLitForBareErr checks if a composite literal (e.g., ErrorWithSuggestion{}) +// has an Err field set to a bare error. +func checkCompositeLitForBareErr( + fset *token.FileSet, + comp *ast.CompositeLit, + relPath, receiverName string, + knownBareErrors map[string]bool, + knownFound *int, + violations *[]string, +) { + for _, elt := range comp.Elts { + kv, ok := elt.(*ast.KeyValueExpr) + if !ok { + continue + } + keyIdent, ok := kv.Key.(*ast.Ident) + if !ok || keyIdent.Name != "Err" { + continue + } + if call, ok := kv.Value.(*ast.CallExpr); ok { + if isBareErrorCall(call) { + pos := fset.Position(comp.Pos()) + key := fmt.Sprintf("%s:%d", relPath, pos.Line) + if knownBareErrors[key] { + (*knownFound)++ + } else { + *violations = append(*violations, fmt.Sprintf( + " %s %s.Run() — ErrorWithSuggestion wrapping bare %s (use a typed sentinel with %%w)", + key, receiverName, callName(call))) + } + } + } + } +} diff --git a/cli/azd/internal/cmd/provision.go b/cli/azd/internal/cmd/provision.go index 26116aa0586..cc07433ba80 100644 --- a/cli/azd/internal/cmd/provision.go +++ b/cli/azd/internal/cmd/provision.go @@ -222,20 +222,24 @@ func (p *ProvisionAction) Run(ctx context.Context) (*actions.ActionResult, error envChanged := false if p.flags.subscription != "" { if existing := p.env.GetSubscriptionId(); existing != "" && existing != p.flags.subscription { - return nil, fmt.Errorf( - "cannot change subscription for existing environment '%s' (current: %s, requested: %s). "+ - "Create a new environment with 'azd env new' instead", - p.env.Name(), existing, p.flags.subscription) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "environment '%s' (current: %s, requested: %s): %w", + p.env.Name(), existing, p.flags.subscription, internal.ErrCannotChangeSubscription), + Suggestion: "Run 'azd env new ' to create a new environment with a different subscription.", + } } p.env.SetSubscriptionId(p.flags.subscription) envChanged = true } if p.flags.location != "" { if existing := p.env.GetLocation(); existing != "" && existing != p.flags.location { - return nil, fmt.Errorf( - "cannot change location for existing environment '%s' (current: %s, requested: %s). "+ - "Create a new environment with 'azd env new' instead", - p.env.Name(), existing, p.flags.location) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "environment '%s' (current: %s, requested: %s): %w", + p.env.Name(), existing, p.flags.location, internal.ErrCannotChangeLocation), + Suggestion: "Run 'azd env new ' to create a new environment with a different location.", + } } p.env.SetLocation(p.flags.location) envChanged = true @@ -268,7 +272,10 @@ func (p *ProvisionAction) Run(ctx context.Context) (*actions.ActionResult, error } if previewMode && len(layers) > 1 { - return nil, fmt.Errorf("--preview cannot be used when provisioning multiple layers. Specify a directly") + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrPreviewMultipleLayers, + Suggestion: "Run 'azd provision --preview ' targeting a single layer.", + } } allSkipped := true diff --git a/cli/azd/internal/cmd/publish.go b/cli/azd/internal/cmd/publish.go index 7ca2fce005b..4d109640e1c 100644 --- a/cli/azd/internal/cmd/publish.go +++ b/cli/azd/internal/cmd/publish.go @@ -5,7 +5,6 @@ package cmd import ( "context" - "errors" "fmt" "io" "log" @@ -160,9 +159,10 @@ func (pa *PublishAction) Run(ctx context.Context) (*actions.ActionResult, error) } if pa.env.GetSubscriptionId() == "" { - return nil, errors.New( - "infrastructure has not been provisioned. Run `azd provision`", - ) + return nil, &internal.ErrorWithSuggestion{ + Err: internal.ErrInfraNotProvisioned, + Suggestion: "Run 'azd provision' to set up infrastructure before publishing.", + } } targetServiceName, err := getTargetServiceName( @@ -180,32 +180,43 @@ func (pa *PublishAction) Run(ctx context.Context) (*actions.ActionResult, error) // Validate that --to requires a specific service if pa.flags.All && pa.flags.To != "" { - return nil, errors.New( - "'--to' cannot be specified when '--all' is set. Specify a specific service by passing a ") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("'--to' cannot be specified when '--all' is set: %w", internal.ErrInvalidFlagCombination), + Suggestion: "Use 'azd publish --to ' to target a specific service.", + } } if targetServiceName == "" && pa.flags.To != "" { - return nil, errors.New( - "'--to' cannot be specified when publishing all services. Specify a specific service by passing a ", - ) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("'--to' requires a specific service: %w", internal.ErrInvalidFlagCombination), + Suggestion: "Use 'azd publish --to ' to target a specific service.", + } } if pa.flags.All && pa.flags.FromPackage != "" { - return nil, errors.New( - "'--from-package' cannot be specified when '--all' is set. Specify a specific service by passing a ") + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "'--from-package' cannot be specified when '--all' is set: %w", + internal.ErrInvalidFlagCombination), + Suggestion: "Use 'azd publish --from-package ' to target a specific service.", + } } if targetServiceName == "" && pa.flags.FromPackage != "" { - return nil, errors.New( - //nolint:lll - "'--from-package' cannot be specified when publishing all services. Specify a specific service by passing a ", - ) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("'--from-package' requires a specific service: %w", internal.ErrInvalidFlagCombination), + Suggestion: "Use 'azd publish --from-package ' to target a specific service.", + } } if pa.flags.FromPackage != "" { if parsedImage, err := docker.ParseContainerImage(pa.flags.FromPackage); err == nil && parsedImage.Registry != "" { - return nil, fmt.Errorf( - "'%s' is already a remote image. Use '--to' flag to specify target", pa.flags.FromPackage) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "'%s' is already a remote image: %w", + pa.flags.FromPackage, internal.ErrInvalidArgValue), + Suggestion: "Use '--to' flag to specify a publish target for remote images.", + } } } diff --git a/cli/azd/internal/cmd/show/show.go b/cli/azd/internal/cmd/show/show.go index e606a196cc3..dbe3e0e0029 100644 --- a/cli/azd/internal/cmd/show/show.go +++ b/cli/azd/internal/cmd/show/show.go @@ -190,9 +190,11 @@ func (s *showAction) Run(ctx context.Context) (*actions.ActionResult, error) { var subId, rgName string if env, err := s.envManager.Get(ctx, environmentName); err != nil { if errors.Is(err, environment.ErrNotFound) && s.flags.EnvironmentName != "" { - return nil, fmt.Errorf( - `"environment '%s' does not exist. You can create it with "azd env new"`, environmentName, - ) + return nil, &internal.ErrorWithSuggestion{ + Err: fmt.Errorf("environment '%s' does not exist: %w", + environmentName, environment.ErrNotFound), + Suggestion: "Run 'azd env new ' to create an environment.", + } } log.Printf("could not load environment: %s, resource ids will not be available", err) } else { diff --git a/cli/azd/internal/errors.go b/cli/azd/internal/errors.go index 9c2423176c2..32ade664236 100644 --- a/cli/azd/internal/errors.go +++ b/cli/azd/internal/errors.go @@ -3,7 +3,11 @@ package internal -import "github.com/azure/azure-dev/cli/azd/pkg/errorhandler" +import ( + "errors" + + "github.com/azure/azure-dev/cli/azd/pkg/errorhandler" +) // ErrorWithSuggestion is a type alias for errorhandler.ErrorWithSuggestion. // The canonical type lives in pkg/errorhandler so it can be used by extensions. @@ -24,3 +28,87 @@ func (et *ErrorWithTraceId) Error() string { func (et *ErrorWithTraceId) Unwrap() error { return et.Err } + +// Command sentinel errors for telemetry classification. +// These enable MapError to produce meaningful ResultCodes instead of falling +// into the opaque "internal.errors_errorString" catch-all bucket. + +// Environment command errors +var ( + ErrNoEnvironmentsFound = errors.New("no environments found") + ErrKeyNotFound = errors.New("key not found in environment values") + ErrNoKeyNameProvided = errors.New("no key name provided") + ErrNoEnvValuesProvided = errors.New("no environment values provided") + ErrInvalidFlagCombination = errors.New("invalid flag combination") +) + +// Deploy command errors +var ( + ErrInfraNotProvisioned = errors.New("infrastructure has not been provisioned") + ErrFromPackageWithAll = errors.New("'--from-package' cannot be specified when '--all' is set") + ErrFromPackageNoService = errors.New( + "'--from-package' cannot be specified when deploying all services") +) + +// Provision command errors +var ( + ErrCannotChangeSubscription = errors.New("cannot change subscription for existing environment") + ErrCannotChangeLocation = errors.New("cannot change location for existing environment") + ErrPreviewMultipleLayers = errors.New("--preview cannot be used when provisioning multiple layers") +) + +// Init command errors +var ( + ErrBranchRequiresTemplate = errors.New( + "using branch argument requires a template argument to be specified") + ErrMultipleInitModes = errors.New( + "only one of init modes: --template, --from-code, or --minimal should be set") +) + +// Auth command errors +var ( + ErrLoginDisabledDelegatedMode = errors.New( + "'azd auth login' is disabled when the auth mode is delegated") +) + +// Cross-command sentinel errors for common error patterns. + +// Argument/flag validation errors +var ( + ErrNoArgsProvided = errors.New("required arguments not provided") + ErrInvalidArgValue = errors.New("invalid argument value") + ErrOperationCancelled = errors.New("operation cancelled by user") +) + +// Config errors +var ( + ErrConfigKeyNotFound = errors.New("config key not found") +) + +// Extension errors +var ( + ErrExtensionNotFound = errors.New("extension not found") + ErrNoExtensionsAvailable = errors.New("no extensions available for operation") + ErrExtensionTokenFailed = errors.New("failed to generate extension token") +) + +// Service/resource errors +var ( + ErrServiceNotFound = errors.New("service not found in project") + ErrResourceNotConfigured = errors.New("required resource not configured") +) + +// Validation errors +var ( + ErrValidationFailed = errors.New("validation failed") +) + +// Unsupported operation errors +var ( + ErrUnsupportedOperation = errors.New("operation not supported") +) + +// MCP errors +var ( + ErrMcpToolsLoadFailed = errors.New("failed to load MCP host tools") +) diff --git a/cli/azd/test/functional/env_test.go b/cli/azd/test/functional/env_test.go index 206aae19686..eba7a7dbdc9 100644 --- a/cli/azd/test/functional/env_test.go +++ b/cli/azd/test/functional/env_test.go @@ -280,7 +280,7 @@ func Test_CLI_Env_Remove(t *testing.T) { // Verify removing when no environment is present res, err := cli.RunCommand(ctx, "env", "remove") require.Error(t, err) - require.Contains(t, res.Stdout, "no environment specified") + require.Contains(t, res.Stdout, "required arguments not provided") // Create two environments envName1 := randomEnvName() @@ -351,7 +351,7 @@ func Test_CLI_Env_GetValue(t *testing.T) { // Test non-existent key res, err := cli.RunCommand(ctx, "env", "get-value", "non_existent_key") require.Error(t, err) - require.Contains(t, res.Stdout, "key 'non_existent_key' not found in the environment values") + require.Contains(t, res.Stdout, "key not found in environment values: 'non_existent_key'") } func envGetValue(ctx context.Context, t *testing.T, cli *azdcli.CLI, key string) string { From 8e3d239cc979cdaaf2c316ff4c35636568f1dde9 Mon Sep 17 00:00:00 2001 From: Rajesh Kamal Date: Tue, 10 Mar 2026 10:40:58 -0700 Subject: [PATCH 2/4] Remove non-actionable suggestions in extensions.go Return base errors instead of wrapping with unhelpful 'This is an internal error...' suggestion text. Addresses JeffreyCA review feedback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/cmd/extensions.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/cli/azd/cmd/extensions.go b/cli/azd/cmd/extensions.go index ecf32f945ed..32a241342f8 100644 --- a/cli/azd/cmd/extensions.go +++ b/cli/azd/cmd/extensions.go @@ -149,10 +149,7 @@ func newExtensionAction( func (a *extensionAction) Run(ctx context.Context) (*actions.ActionResult, error) { extensionId, has := a.cmd.Annotations["extension.id"] if !has { - return nil, &internal.ErrorWithSuggestion{ - Err: internal.ErrExtensionNotFound, - Suggestion: "This is an internal error — the extension annotation is missing from the command.", - } + return nil, internal.ErrExtensionNotFound } extension, err := a.extensionManager.GetInstalled(extensions.FilterOptions{ @@ -231,10 +228,10 @@ func (a *extensionAction) Run(ctx context.Context) (*actions.ActionResult, error jwtToken, err := grpcserver.GenerateExtensionToken(extension, serverInfo) if err != nil { - return nil, &internal.ErrorWithSuggestion{ - Err: internal.ErrExtensionTokenFailed, - Suggestion: "This is an internal error generating the JWT token for extension communication.", - } + return nil, fmt.Errorf( + "generating extension token: %w", + internal.ErrExtensionTokenFailed, + ) } allEnv = append(allEnv, From 935b93b00de3867f88c88988b268d81b18ee4f5e Mon Sep 17 00:00:00 2001 From: Rajesh Kamal Date: Tue, 10 Mar 2026 11:00:57 -0700 Subject: [PATCH 3/4] Revert ErrorWithSuggestion unwrap logic Restore original error.suggestion + error.type classification. Wei confirmed the empty error.type was a backend GDPR classification issue, now fixed. The original design is correct: ErrorWithSuggestion produces error.suggestion as ResultCode with the inner error type in the error.type attribute. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/internal/cmd/errors.go | 116 +++++++++++++--------------- cli/azd/internal/cmd/errors_test.go | 80 ++++++------------- 2 files changed, 78 insertions(+), 118 deletions(-) diff --git a/cli/azd/internal/cmd/errors.go b/cli/azd/internal/cmd/errors.go index ef94f139782..db838b30724 100644 --- a/cli/azd/internal/cmd/errors.go +++ b/cli/azd/internal/cmd/errors.go @@ -61,21 +61,15 @@ func MapError(err error, span tracing.Span) { var loginErr *auth.ReLoginRequiredError var updateErr *update.UpdateError - // If the error is wrapped in ErrorWithSuggestion, unwrap it first so the inner error - // can be classified by its actual type (ResponseError, AzureDeploymentError, etc.) - // instead of being bucketed as the opaque "error.suggestion". - classifyErr := err - if errors.As(err, &errWithSuggestion) { - if inner := errWithSuggestion.Unwrap(); inner != nil { - classifyErr = inner - } - } - - if errors.As(classifyErr, &updateErr) { + if errors.As(err, &updateErr) { errCode = updateErr.Code - } else if errors.As(classifyErr, &loginErr) { + } else if errors.As(err, &loginErr) { errCode = "auth.login_required" - } else if errors.As(classifyErr, &respErr) { + } else if errors.As(err, &errWithSuggestion) { + errCode = "error.suggestion" + errType := errorType(errWithSuggestion.Unwrap()) + span.SetAttributes(fields.ErrType.String(errType)) + } else if errors.As(err, &respErr) { serviceName := "other" statusCode := -1 errDetails = append(errDetails, fields.ServiceErrorCode.String(respErr.ErrorCode)) @@ -96,7 +90,7 @@ func MapError(err error, span tracing.Span) { } errCode = fmt.Sprintf("service.%s.%d", serviceName, statusCode) - } else if errors.As(classifyErr, &armDeployErr) { + } else if errors.As(err, &armDeployErr) { errDetails = append(errDetails, fields.ServiceName.String("arm")) codes := []*deploymentErrorCode{} var collect func(details []*azapi.DeploymentErrorLine, frame int) @@ -130,7 +124,7 @@ func MapError(err error, span tracing.Span) { operation = "deployment" } errCode = fmt.Sprintf("service.arm.%s.failed", operation) - } else if errors.As(classifyErr, &extServiceErr) { + } else if errors.As(err, &extServiceErr) { // Handle structured service errors from extensions. // Emit whatever details are available rather than requiring all fields. serviceName := "" @@ -161,7 +155,7 @@ func MapError(err error, span tracing.Span) { default: errCode = "ext.service.unknown.failed" } - } else if errors.As(classifyErr, &extLocalErr) { + } else if errors.As(err, &extLocalErr) { domain := string(azdext.NormalizeLocalErrorCategory(extLocalErr.Category)) code := normalizeCodeSegment(extLocalErr.Code, "failed") @@ -171,9 +165,9 @@ func MapError(err error, span tracing.Span) { ) errCode = fmt.Sprintf("ext.%s.%s", domain, code) - } else if errors.As(classifyErr, &extensionRunErr) { + } else if errors.As(err, &extensionRunErr) { errCode = "ext.run.failed" - } else if errors.As(classifyErr, &toolExecErr) { + } else if errors.As(err, &toolExecErr) { toolName := "other" cmdName := cmdAsName(toolExecErr.Cmd) if cmdName != "" { @@ -185,7 +179,7 @@ func MapError(err error, span tracing.Span) { fields.ToolName.String(toolName)) errCode = fmt.Sprintf("tool.%s.failed", toolName) - } else if errors.As(classifyErr, &toolCheckErr) { + } else if errors.As(err, &toolCheckErr) { if len(toolCheckErr.ToolNames) == 1 { toolName := toolCheckErr.ToolNames[0] errCode = fmt.Sprintf("tool.%s.missing", toolName) @@ -194,7 +188,7 @@ func MapError(err error, span tracing.Span) { errCode = "tool.multiple.missing" errDetails = append(errDetails, fields.ToolName.String(strings.Join(toolCheckErr.ToolNames, ","))) } - } else if errors.As(classifyErr, &authFailedErr) { + } else if errors.As(err, &authFailedErr) { errDetails = append(errDetails, fields.ServiceName.String("aad")) if authFailedErr.Parsed != nil { codes := make([]string, 0, len(authFailedErr.Parsed.ErrorCodes)) @@ -208,83 +202,83 @@ func MapError(err error, span tracing.Span) { fields.ServiceCorrelationId.String(authFailedErr.Parsed.CorrelationId)) } errCode = "service.aad.failed" - } else if errors.Is(classifyErr, terminal.InterruptErr) { + } else if errors.Is(err, terminal.InterruptErr) { errCode = "user.canceled" - } else if errors.Is(classifyErr, context.Canceled) { + } else if errors.Is(err, context.Canceled) { errCode = "user.canceled" - } else if errors.Is(classifyErr, context.DeadlineExceeded) { + } else if errors.Is(err, context.DeadlineExceeded) { errCode = "internal.timeout" - } else if errors.Is(classifyErr, auth.ErrNoCurrentUser) { + } else if errors.Is(err, auth.ErrNoCurrentUser) { errCode = "auth.not_logged_in" - } else if errors.Is(classifyErr, consent.ErrToolExecutionDenied) { + } else if errors.Is(err, consent.ErrToolExecutionDenied) { errCode = "user.tool_denied" - } else if errors.Is(classifyErr, git.ErrNotRepository) { + } else if errors.Is(err, git.ErrNotRepository) { errCode = "internal.not_git_repo" - } else if errors.Is(classifyErr, azapi.ErrPreviewNotSupported) { + } else if errors.Is(err, azapi.ErrPreviewNotSupported) { errCode = "internal.preview_not_supported" - } else if errors.Is(classifyErr, provisioning.ErrBindMountOperationDisabled) { + } else if errors.Is(err, provisioning.ErrBindMountOperationDisabled) { errCode = "internal.bind_mount_disabled" - } else if errors.Is(classifyErr, update.ErrNeedsElevation) { + } else if errors.Is(err, update.ErrNeedsElevation) { errCode = "update.elevationRequired" - } else if errors.Is(classifyErr, pipeline.ErrRemoteHostIsNotAzDo) { + } else if errors.Is(err, pipeline.ErrRemoteHostIsNotAzDo) { errCode = "internal.remote_not_azdo" - } else if errors.Is(classifyErr, internal.ErrInfraNotProvisioned) { + } else if errors.Is(err, internal.ErrInfraNotProvisioned) { errCode = "internal.infra_not_provisioned" - } else if errors.Is(classifyErr, internal.ErrFromPackageWithAll) || - errors.Is(classifyErr, internal.ErrFromPackageNoService) { + } else if errors.Is(err, internal.ErrFromPackageWithAll) || + errors.Is(err, internal.ErrFromPackageNoService) { errCode = "internal.invalid_flag_combination" - } else if errors.Is(classifyErr, internal.ErrCannotChangeSubscription) { + } else if errors.Is(err, internal.ErrCannotChangeSubscription) { errCode = "internal.cannot_change_subscription" - } else if errors.Is(classifyErr, internal.ErrCannotChangeLocation) { + } else if errors.Is(err, internal.ErrCannotChangeLocation) { errCode = "internal.cannot_change_location" - } else if errors.Is(classifyErr, internal.ErrPreviewMultipleLayers) { + } else if errors.Is(err, internal.ErrPreviewMultipleLayers) { errCode = "internal.preview_multiple_layers" - } else if errors.Is(classifyErr, internal.ErrNoKeyNameProvided) || - errors.Is(classifyErr, internal.ErrNoEnvValuesProvided) || - errors.Is(classifyErr, internal.ErrInvalidFlagCombination) { + } else if errors.Is(err, internal.ErrNoKeyNameProvided) || + errors.Is(err, internal.ErrNoEnvValuesProvided) || + errors.Is(err, internal.ErrInvalidFlagCombination) { errCode = "internal.invalid_args" - } else if errors.Is(classifyErr, internal.ErrKeyNotFound) { + } else if errors.Is(err, internal.ErrKeyNotFound) { errCode = "internal.key_not_found" - } else if errors.Is(classifyErr, internal.ErrNoEnvironmentsFound) { + } else if errors.Is(err, internal.ErrNoEnvironmentsFound) { errCode = "internal.no_environments_found" - } else if errors.Is(classifyErr, internal.ErrLoginDisabledDelegatedMode) { + } else if errors.Is(err, internal.ErrLoginDisabledDelegatedMode) { errCode = "auth.login_disabled_delegated" - } else if errors.Is(classifyErr, internal.ErrBranchRequiresTemplate) || - errors.Is(classifyErr, internal.ErrMultipleInitModes) { + } else if errors.Is(err, internal.ErrBranchRequiresTemplate) || + errors.Is(err, internal.ErrMultipleInitModes) { errCode = "internal.invalid_args" - } else if errors.Is(classifyErr, environment.ErrNotFound) { + } else if errors.Is(err, environment.ErrNotFound) { errCode = "internal.env_not_found" - } else if errors.Is(classifyErr, azdcontext.ErrNoProject) { + } else if errors.Is(err, azdcontext.ErrNoProject) { errCode = "internal.no_project" - } else if errors.Is(classifyErr, internal.ErrNoArgsProvided) || - errors.Is(classifyErr, internal.ErrInvalidArgValue) { + } else if errors.Is(err, internal.ErrNoArgsProvided) || + errors.Is(err, internal.ErrInvalidArgValue) { errCode = "internal.invalid_args" - } else if errors.Is(classifyErr, internal.ErrConfigKeyNotFound) { + } else if errors.Is(err, internal.ErrConfigKeyNotFound) { errCode = "internal.config_key_not_found" - } else if errors.Is(classifyErr, internal.ErrExtensionNotFound) { + } else if errors.Is(err, internal.ErrExtensionNotFound) { errCode = "internal.extension_not_found" - } else if errors.Is(classifyErr, internal.ErrServiceNotFound) { + } else if errors.Is(err, internal.ErrServiceNotFound) { errCode = "internal.service_not_found" - } else if errors.Is(classifyErr, internal.ErrNoExtensionsAvailable) { + } else if errors.Is(err, internal.ErrNoExtensionsAvailable) { errCode = "internal.no_extensions_available" - } else if errors.Is(classifyErr, internal.ErrValidationFailed) { + } else if errors.Is(err, internal.ErrValidationFailed) { errCode = "internal.validation_failed" - } else if errors.Is(classifyErr, internal.ErrUnsupportedOperation) { + } else if errors.Is(err, internal.ErrUnsupportedOperation) { errCode = "internal.unsupported_operation" - } else if errors.Is(classifyErr, internal.ErrExtensionTokenFailed) { + } else if errors.Is(err, internal.ErrExtensionTokenFailed) { errCode = "internal.extension_error" - } else if errors.Is(classifyErr, internal.ErrMcpToolsLoadFailed) { + } else if errors.Is(err, internal.ErrMcpToolsLoadFailed) { errCode = "internal.mcp_error" - } else if errors.Is(classifyErr, internal.ErrResourceNotConfigured) { + } else if errors.Is(err, internal.ErrResourceNotConfigured) { errCode = "internal.resource_not_found" - } else if errors.Is(classifyErr, internal.ErrOperationCancelled) { + } else if errors.Is(err, internal.ErrOperationCancelled) { errCode = "internal.operation_cancelled" - } else if isNetworkError(classifyErr) { + } else if isNetworkError(err) { errCode = "internal.network" - errType := errorType(classifyErr) + errType := errorType(err) span.SetAttributes(fields.ErrType.String(errType)) } else { - errType := errorType(classifyErr) + errType := errorType(err) span.SetAttributes(fields.ErrType.String(errType)) errCode = fmt.Sprintf("internal.%s", strings.ReplaceAll(strings.ReplaceAll(errType, ".", "_"), "*", "")) diff --git a/cli/azd/internal/cmd/errors_test.go b/cli/azd/internal/cmd/errors_test.go index c765c2d7911..965933e717a 100644 --- a/cli/azd/internal/cmd/errors_test.go +++ b/cli/azd/internal/cmd/errors_test.go @@ -401,13 +401,9 @@ func Test_MapError(t *testing.T) { }, Suggestion: "Request a quota increase in the Azure portal.", }, - wantErrReason: "service.arm.429", + wantErrReason: "error.suggestion", wantErrDetails: []attribute.KeyValue{ - fields.ErrorKey(fields.ServiceName.Key).String("arm"), - fields.ErrorKey(fields.ServiceHost.Key).String("management.azure.com"), - fields.ErrorKey(fields.ServiceMethod.Key).String("POST"), - fields.ErrorKey(fields.ServiceErrorCode.Key).String("QuotaExceeded"), - fields.ErrorKey(fields.ServiceStatusCode.Key).Int(429), + fields.ErrType.String("*exported.ResponseError"), }, }, { @@ -416,7 +412,7 @@ func Test_MapError(t *testing.T) { Err: errors.New("something failed"), Suggestion: "Try again later.", }, - wantErrReason: "internal.errors_errorString", + wantErrReason: "error.suggestion", wantErrDetails: []attribute.KeyValue{ fields.ErrType.String("*errors.errorString"), }, @@ -567,9 +563,8 @@ func Test_MapError(t *testing.T) { // Test cases that intentionally produce errors_errorString (the catch-all bucket). // Any NEW test case that produces this is a signal that a typed sentinel is needed. allowedCatchAll := map[string]bool{ - "WithNilError": true, - "WithOtherError": true, - "WithSuggestionWrappingPlainError": true, + "WithNilError": true, + "WithOtherError": true, } for _, tt := range tests { @@ -591,20 +586,18 @@ func Test_MapError(t *testing.T) { } } -// TestMapError_ErrorWithSuggestionUnwrapsInnerError verifies that ErrorWithSuggestion no longer -// masks the inner error's classification. Previously, any error wrapped in ErrorWithSuggestion -// was reported as "error.suggestion" in telemetry, losing ~3,600 hits of actionable error data. -// After the fix, the inner error is unwrapped and classified by its actual type. -func TestMapError_ErrorWithSuggestionUnwrapsInnerError(t *testing.T) { +// TestMapError_ErrorWithSuggestionSetsErrorType verifies that ErrorWithSuggestion errors +// are classified as "error.suggestion" in telemetry, with the inner error type +// recorded in the error.type span attribute for detailed analysis. +func TestMapError_ErrorWithSuggestionSetsErrorType(t *testing.T) { tests := []struct { - name string - err error - wantErrCode string - wantNotCode string // the code it would have been before the fix - wantAttribute *attribute.KeyValue + name string + err error + wantErrCode string + wantErrType string }{ { - name: "ResponseError_classified_by_inner_type", + name: "ResponseError_inner_type_recorded", err: &internal.ErrorWithSuggestion{ Err: &azcore.ResponseError{ ErrorCode: "QuotaExceeded", @@ -616,42 +609,17 @@ func TestMapError_ErrorWithSuggestionUnwrapsInnerError(t *testing.T) { }, Suggestion: "Request a quota increase.", }, - wantErrCode: "service.arm.429", - wantNotCode: "error.suggestion", + wantErrCode: "error.suggestion", + wantErrType: "*exported.ResponseError", }, { - name: "ArmDeploymentError_classified_by_inner_type", - err: &internal.ErrorWithSuggestion{ - Err: &azapi.AzureDeploymentError{ - Operation: azapi.DeploymentOperationDeploy, - Details: &azapi.DeploymentErrorLine{Code: "Conflict"}, - }, - Suggestion: "Check for existing resources.", - }, - wantErrCode: "service.arm.deployment.failed", - wantNotCode: "error.suggestion", - }, - { - name: "ToolExitError_classified_by_inner_type", - err: &internal.ErrorWithSuggestion{ - Err: &exec.ExitError{Cmd: "docker", ExitCode: 1}, - Suggestion: "Run docker login.", - }, - wantErrCode: "tool.docker.failed", - wantNotCode: "error.suggestion", - }, - { - name: "PlainError_falls_through_to_internal", + name: "PlainError_inner_type_recorded", err: &internal.ErrorWithSuggestion{ Err: errors.New("unknown failure"), Suggestion: "Try again.", }, - wantErrCode: "internal.errors_errorString", - wantNotCode: "error.suggestion", - wantAttribute: func() *attribute.KeyValue { - kv := fields.ErrType.String("*errors.errorString") - return &kv - }(), + wantErrCode: "error.suggestion", + wantErrType: "*errors.errorString", }, } @@ -661,13 +629,11 @@ func TestMapError_ErrorWithSuggestionUnwrapsInnerError(t *testing.T) { MapError(tt.err, span) require.Equal(t, tt.wantErrCode, span.Status.Description, - "should classify by inner error type, not as opaque error.suggestion") - require.NotEqual(t, tt.wantNotCode, span.Status.Description, - "should NOT produce the old opaque code") + "ErrorWithSuggestion should produce error.suggestion") - if tt.wantAttribute != nil { - require.Contains(t, span.Attributes, *tt.wantAttribute) - } + wantAttr := fields.ErrType.String(tt.wantErrType) + require.Contains(t, span.Attributes, wantAttr, + "error.type attribute should record the inner error type") }) } } From 3c246db7e681857b1f6ac82102516bd320e8ac5e Mon Sep 17 00:00:00 2001 From: Rajesh Kamal Date: Tue, 10 Mar 2026 17:09:45 -0700 Subject: [PATCH 4/4] Remove dead sentinel checks and classify via ErrorWithSuggestion Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cli/azd/internal/cmd/errors.go | 116 +++++---- cli/azd/internal/cmd/errors_test.go | 391 ++++++++++++++++++++++------ 2 files changed, 373 insertions(+), 134 deletions(-) diff --git a/cli/azd/internal/cmd/errors.go b/cli/azd/internal/cmd/errors.go index db838b30724..fa468defbda 100644 --- a/cli/azd/internal/cmd/errors.go +++ b/cli/azd/internal/cmd/errors.go @@ -67,8 +67,13 @@ func MapError(err error, span tracing.Span) { errCode = "auth.login_required" } else if errors.As(err, &errWithSuggestion) { errCode = "error.suggestion" - errType := errorType(errWithSuggestion.Unwrap()) - span.SetAttributes(fields.ErrType.String(errType)) + inner := errWithSuggestion.Unwrap() + if code := classifySentinel(inner); code != "" { + span.SetAttributes(fields.ErrType.String(code)) + } else { + span.SetAttributes( + fields.ErrType.String(errorType(inner))) + } } else if errors.As(err, &respErr) { serviceName := "other" statusCode := -1 @@ -222,57 +227,10 @@ func MapError(err error, span tracing.Span) { errCode = "update.elevationRequired" } else if errors.Is(err, pipeline.ErrRemoteHostIsNotAzDo) { errCode = "internal.remote_not_azdo" - } else if errors.Is(err, internal.ErrInfraNotProvisioned) { - errCode = "internal.infra_not_provisioned" - } else if errors.Is(err, internal.ErrFromPackageWithAll) || - errors.Is(err, internal.ErrFromPackageNoService) { - errCode = "internal.invalid_flag_combination" - } else if errors.Is(err, internal.ErrCannotChangeSubscription) { - errCode = "internal.cannot_change_subscription" - } else if errors.Is(err, internal.ErrCannotChangeLocation) { - errCode = "internal.cannot_change_location" - } else if errors.Is(err, internal.ErrPreviewMultipleLayers) { - errCode = "internal.preview_multiple_layers" - } else if errors.Is(err, internal.ErrNoKeyNameProvided) || - errors.Is(err, internal.ErrNoEnvValuesProvided) || - errors.Is(err, internal.ErrInvalidFlagCombination) { - errCode = "internal.invalid_args" - } else if errors.Is(err, internal.ErrKeyNotFound) { - errCode = "internal.key_not_found" - } else if errors.Is(err, internal.ErrNoEnvironmentsFound) { - errCode = "internal.no_environments_found" - } else if errors.Is(err, internal.ErrLoginDisabledDelegatedMode) { - errCode = "auth.login_disabled_delegated" - } else if errors.Is(err, internal.ErrBranchRequiresTemplate) || - errors.Is(err, internal.ErrMultipleInitModes) { - errCode = "internal.invalid_args" - } else if errors.Is(err, environment.ErrNotFound) { - errCode = "internal.env_not_found" - } else if errors.Is(err, azdcontext.ErrNoProject) { - errCode = "internal.no_project" - } else if errors.Is(err, internal.ErrNoArgsProvided) || - errors.Is(err, internal.ErrInvalidArgValue) { - errCode = "internal.invalid_args" - } else if errors.Is(err, internal.ErrConfigKeyNotFound) { - errCode = "internal.config_key_not_found" } else if errors.Is(err, internal.ErrExtensionNotFound) { errCode = "internal.extension_not_found" - } else if errors.Is(err, internal.ErrServiceNotFound) { - errCode = "internal.service_not_found" - } else if errors.Is(err, internal.ErrNoExtensionsAvailable) { - errCode = "internal.no_extensions_available" - } else if errors.Is(err, internal.ErrValidationFailed) { - errCode = "internal.validation_failed" - } else if errors.Is(err, internal.ErrUnsupportedOperation) { - errCode = "internal.unsupported_operation" } else if errors.Is(err, internal.ErrExtensionTokenFailed) { errCode = "internal.extension_error" - } else if errors.Is(err, internal.ErrMcpToolsLoadFailed) { - errCode = "internal.mcp_error" - } else if errors.Is(err, internal.ErrResourceNotConfigured) { - errCode = "internal.resource_not_found" - } else if errors.Is(err, internal.ErrOperationCancelled) { - errCode = "internal.operation_cancelled" } else if isNetworkError(err) { errCode = "internal.network" errType := errorType(err) @@ -295,6 +253,66 @@ func MapError(err error, span tracing.Span) { span.SetStatus(codes.Error, errCode) } +// classifySentinel checks if the error matches a known sentinel +// and returns the corresponding telemetry code, or "" if no match. +func classifySentinel(err error) string { + switch { + case errors.Is(err, internal.ErrInfraNotProvisioned): + return "internal.infra_not_provisioned" + case errors.Is(err, internal.ErrFromPackageWithAll), + errors.Is(err, internal.ErrFromPackageNoService): + return "internal.invalid_flag_combination" + case errors.Is(err, internal.ErrCannotChangeSubscription): + return "internal.cannot_change_subscription" + case errors.Is(err, internal.ErrCannotChangeLocation): + return "internal.cannot_change_location" + case errors.Is(err, internal.ErrPreviewMultipleLayers): + return "internal.preview_multiple_layers" + case errors.Is(err, internal.ErrNoKeyNameProvided), + errors.Is(err, internal.ErrNoEnvValuesProvided), + errors.Is(err, internal.ErrInvalidFlagCombination): + return "internal.invalid_args" + case errors.Is(err, internal.ErrKeyNotFound): + return "internal.key_not_found" + case errors.Is(err, internal.ErrNoEnvironmentsFound): + return "internal.no_environments_found" + case errors.Is(err, internal.ErrLoginDisabledDelegatedMode): + return "auth.login_disabled_delegated" + case errors.Is(err, internal.ErrBranchRequiresTemplate), + errors.Is(err, internal.ErrMultipleInitModes): + return "internal.invalid_args" + case errors.Is(err, environment.ErrNotFound): + return "internal.env_not_found" + case errors.Is(err, azdcontext.ErrNoProject): + return "internal.no_project" + case errors.Is(err, internal.ErrNoArgsProvided), + errors.Is(err, internal.ErrInvalidArgValue): + return "internal.invalid_args" + case errors.Is(err, internal.ErrConfigKeyNotFound): + return "internal.config_key_not_found" + case errors.Is(err, internal.ErrExtensionNotFound): + return "internal.extension_not_found" + case errors.Is(err, internal.ErrServiceNotFound): + return "internal.service_not_found" + case errors.Is(err, internal.ErrNoExtensionsAvailable): + return "internal.no_extensions_available" + case errors.Is(err, internal.ErrValidationFailed): + return "internal.validation_failed" + case errors.Is(err, internal.ErrUnsupportedOperation): + return "internal.unsupported_operation" + case errors.Is(err, internal.ErrExtensionTokenFailed): + return "internal.extension_error" + case errors.Is(err, internal.ErrMcpToolsLoadFailed): + return "internal.mcp_error" + case errors.Is(err, internal.ErrResourceNotConfigured): + return "internal.resource_not_found" + case errors.Is(err, internal.ErrOperationCancelled): + return "internal.operation_cancelled" + default: + return "" + } +} + // errorType returns the type name of the given error, unwrapping as needed to find the root cause(s). func errorType(err error) string { if err == nil { diff --git a/cli/azd/internal/cmd/errors_test.go b/cli/azd/internal/cmd/errors_test.go index 965933e717a..a33c1ce27ac 100644 --- a/cli/azd/internal/cmd/errors_test.go +++ b/cli/azd/internal/cmd/errors_test.go @@ -417,147 +417,352 @@ func Test_MapError(t *testing.T) { fields.ErrType.String("*errors.errorString"), }, }, - // Sentinel error test cases — verify typed errors produce meaningful ResultCodes + // Sentinel error test cases — verify typed errors wrapped in + // ErrorWithSuggestion produce error.suggestion ResultCode with + // the sentinel code in error.type via classifySentinel. { - name: "WithErrNoProject", - err: azdcontext.ErrNoProject, - wantErrReason: "internal.no_project", + name: "WithErrNoProject", + err: &internal.ErrorWithSuggestion{ + Err: azdcontext.ErrNoProject, + Suggestion: "Run azd init.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String("internal.no_project"), + }, }, { - name: "WithErrEnvNotFound", - err: fmt.Errorf("environment 'dev' does not exist: %w", environment.ErrNotFound), - wantErrReason: "internal.env_not_found", + name: "WithErrEnvNotFound", + err: &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "environment 'dev' does not exist: %w", + environment.ErrNotFound), + Suggestion: "Run azd env new.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String("internal.env_not_found"), + }, }, { - name: "WithErrInfraNotProvisioned", - err: fmt.Errorf("run `azd provision`: %w", internal.ErrInfraNotProvisioned), - wantErrReason: "internal.infra_not_provisioned", + name: "WithErrInfraNotProvisioned", + err: &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "run azd provision: %w", + internal.ErrInfraNotProvisioned), + Suggestion: "Run azd provision.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "internal.infra_not_provisioned"), + }, }, { - name: "WithErrFromPackageWithAll", - err: fmt.Errorf("specify a service: %w", internal.ErrFromPackageWithAll), - wantErrReason: "internal.invalid_flag_combination", + name: "WithErrFromPackageWithAll", + err: &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "specify a service: %w", + internal.ErrFromPackageWithAll), + Suggestion: "Specify a service.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "internal.invalid_flag_combination"), + }, }, { - name: "WithErrFromPackageNoService", - err: fmt.Errorf("specify a service: %w", internal.ErrFromPackageNoService), - wantErrReason: "internal.invalid_flag_combination", + name: "WithErrFromPackageNoService", + err: &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "specify a service: %w", + internal.ErrFromPackageNoService), + Suggestion: "Specify a service.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "internal.invalid_flag_combination"), + }, }, { - name: "WithErrCannotChangeSubscription", - err: fmt.Errorf("env 'dev': %w", internal.ErrCannotChangeSubscription), - wantErrReason: "internal.cannot_change_subscription", + name: "WithErrCannotChangeSubscription", + err: &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "env 'dev': %w", + internal.ErrCannotChangeSubscription), + Suggestion: "Run azd env new.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "internal.cannot_change_subscription"), + }, }, { - name: "WithErrCannotChangeLocation", - err: fmt.Errorf("env 'dev': %w", internal.ErrCannotChangeLocation), - wantErrReason: "internal.cannot_change_location", + name: "WithErrCannotChangeLocation", + err: &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "env 'dev': %w", + internal.ErrCannotChangeLocation), + Suggestion: "Run azd env new.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "internal.cannot_change_location"), + }, }, { - name: "WithErrPreviewMultipleLayers", - err: fmt.Errorf("specify a layer: %w", internal.ErrPreviewMultipleLayers), - wantErrReason: "internal.preview_multiple_layers", + name: "WithErrPreviewMultipleLayers", + err: &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "specify a layer: %w", + internal.ErrPreviewMultipleLayers), + Suggestion: "Specify a single layer.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "internal.preview_multiple_layers"), + }, }, { - name: "WithErrNoKeyNameProvided", - err: internal.ErrNoKeyNameProvided, - wantErrReason: "internal.invalid_args", + name: "WithErrNoKeyNameProvided", + err: &internal.ErrorWithSuggestion{ + Err: internal.ErrNoKeyNameProvided, + Suggestion: "Specify a key.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String("internal.invalid_args"), + }, }, { - name: "WithErrNoEnvValuesProvided", - err: fmt.Errorf("use key=value: %w", internal.ErrNoEnvValuesProvided), - wantErrReason: "internal.invalid_args", + name: "WithErrNoEnvValuesProvided", + err: &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "use key=value: %w", + internal.ErrNoEnvValuesProvided), + Suggestion: "Use key=value pairs.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String("internal.invalid_args"), + }, }, { - name: "WithErrInvalidFlagCombination", - err: fmt.Errorf("cannot combine flags: %w", internal.ErrInvalidFlagCombination), - wantErrReason: "internal.invalid_args", + name: "WithErrInvalidFlagCombination", + err: &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "cannot combine flags: %w", + internal.ErrInvalidFlagCombination), + Suggestion: "Choose one flag.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String("internal.invalid_args"), + }, }, { - name: "WithErrKeyNotFound", - err: fmt.Errorf("%w: 'MY_KEY'", internal.ErrKeyNotFound), - wantErrReason: "internal.key_not_found", + name: "WithErrKeyNotFound", + err: &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "%w: 'MY_KEY'", internal.ErrKeyNotFound), + Suggestion: "Run azd env get-values.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "internal.key_not_found"), + }, }, { - name: "WithErrNoEnvironmentsFound", - err: fmt.Errorf("create one with azd env new: %w", internal.ErrNoEnvironmentsFound), - wantErrReason: "internal.no_environments_found", + name: "WithErrNoEnvironmentsFound", + err: &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "create one: %w", + internal.ErrNoEnvironmentsFound), + Suggestion: "Run azd env new.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "internal.no_environments_found"), + }, }, { - name: "WithErrLoginDisabledDelegatedMode", - err: fmt.Errorf("current mode: az_delegated: %w", internal.ErrLoginDisabledDelegatedMode), - wantErrReason: "auth.login_disabled_delegated", + name: "WithErrLoginDisabledDelegatedMode", + err: &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "current mode: %w", + internal.ErrLoginDisabledDelegatedMode), + Suggestion: "Use delegated identity.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "auth.login_disabled_delegated"), + }, }, { - name: "WithErrBranchRequiresTemplate", - err: fmt.Errorf("use --template: %w", internal.ErrBranchRequiresTemplate), - wantErrReason: "internal.invalid_args", + name: "WithErrBranchRequiresTemplate", + err: &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "use --template: %w", + internal.ErrBranchRequiresTemplate), + Suggestion: "Add --template.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String("internal.invalid_args"), + }, }, { - name: "WithErrMultipleInitModes", - err: internal.ErrMultipleInitModes, - wantErrReason: "internal.invalid_args", + name: "WithErrMultipleInitModes", + err: &internal.ErrorWithSuggestion{ + Err: internal.ErrMultipleInitModes, + Suggestion: "Choose one mode.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String("internal.invalid_args"), + }, }, - // New sentinels — batch 2 (54 bare-error fixes) + // Sentinels — batch 2 (54 bare-error fixes) { - name: "WithErrNoArgsProvided", - err: internal.ErrNoArgsProvided, - wantErrReason: "internal.invalid_args", + name: "WithErrNoArgsProvided", + err: &internal.ErrorWithSuggestion{ + Err: internal.ErrNoArgsProvided, + Suggestion: "Provide required args.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String("internal.invalid_args"), + }, }, { - name: "WithErrInvalidArgValue", - err: internal.ErrInvalidArgValue, - wantErrReason: "internal.invalid_args", + name: "WithErrInvalidArgValue", + err: &internal.ErrorWithSuggestion{ + Err: internal.ErrInvalidArgValue, + Suggestion: "Check the value.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String("internal.invalid_args"), + }, }, { - name: "WithErrOperationCancelled", - err: internal.ErrOperationCancelled, - wantErrReason: "internal.operation_cancelled", + name: "WithErrOperationCancelled", + err: &internal.ErrorWithSuggestion{ + Err: internal.ErrOperationCancelled, + Suggestion: "Try again.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "internal.operation_cancelled"), + }, }, { - name: "WithErrConfigKeyNotFound", - err: internal.ErrConfigKeyNotFound, - wantErrReason: "internal.config_key_not_found", + name: "WithErrConfigKeyNotFound", + err: &internal.ErrorWithSuggestion{ + Err: internal.ErrConfigKeyNotFound, + Suggestion: "Run azd config show.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "internal.config_key_not_found"), + }, }, + // ErrExtensionNotFound: kept naked — matches real + // usage in extensions.go:152 { name: "WithErrExtensionNotFound", err: internal.ErrExtensionNotFound, wantErrReason: "internal.extension_not_found", }, { - name: "WithErrNoExtensionsAvailable", - err: internal.ErrNoExtensionsAvailable, - wantErrReason: "internal.no_extensions_available", + name: "WithErrNoExtensionsAvailable", + err: &internal.ErrorWithSuggestion{ + Err: internal.ErrNoExtensionsAvailable, + Suggestion: "Run azd extension list.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "internal.no_extensions_available"), + }, }, + // ErrExtensionTokenFailed: kept naked with %w — + // matches real usage in extensions.go:233 { - name: "WithErrExtensionTokenFailed", - err: internal.ErrExtensionTokenFailed, + name: "WithErrExtensionTokenFailed", + err: fmt.Errorf( + "generating token: %w", + internal.ErrExtensionTokenFailed), wantErrReason: "internal.extension_error", }, { - name: "WithErrServiceNotFound", - err: internal.ErrServiceNotFound, - wantErrReason: "internal.service_not_found", + name: "WithErrServiceNotFound", + err: &internal.ErrorWithSuggestion{ + Err: internal.ErrServiceNotFound, + Suggestion: "Check azure.yaml.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "internal.service_not_found"), + }, }, { - name: "WithErrResourceNotConfigured", - err: internal.ErrResourceNotConfigured, - wantErrReason: "internal.resource_not_found", + name: "WithErrResourceNotConfigured", + err: &internal.ErrorWithSuggestion{ + Err: internal.ErrResourceNotConfigured, + Suggestion: "Run azd provision.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "internal.resource_not_found"), + }, }, { - name: "WithErrValidationFailed", - err: internal.ErrValidationFailed, - wantErrReason: "internal.validation_failed", + name: "WithErrValidationFailed", + err: &internal.ErrorWithSuggestion{ + Err: internal.ErrValidationFailed, + Suggestion: "Fix validation errors.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "internal.validation_failed"), + }, }, { - name: "WithErrUnsupportedOperation", - err: internal.ErrUnsupportedOperation, - wantErrReason: "internal.unsupported_operation", + name: "WithErrUnsupportedOperation", + err: &internal.ErrorWithSuggestion{ + Err: internal.ErrUnsupportedOperation, + Suggestion: "Check supported options.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String( + "internal.unsupported_operation"), + }, }, { - name: "WithErrMcpToolsLoadFailed", - err: internal.ErrMcpToolsLoadFailed, - wantErrReason: "internal.mcp_error", + name: "WithErrMcpToolsLoadFailed", + err: &internal.ErrorWithSuggestion{ + Err: internal.ErrMcpToolsLoadFailed, + Suggestion: "Check MCP config.", + }, + wantErrReason: "error.suggestion", + wantErrDetails: []attribute.KeyValue{ + fields.ErrType.String("internal.mcp_error"), + }, }, } // Test cases that intentionally produce errors_errorString (the catch-all bucket). @@ -589,6 +794,8 @@ func Test_MapError(t *testing.T) { // TestMapError_ErrorWithSuggestionSetsErrorType verifies that ErrorWithSuggestion errors // are classified as "error.suggestion" in telemetry, with the inner error type // recorded in the error.type span attribute for detailed analysis. +// When the inner error matches a known sentinel, the descriptive code is used +// instead of the raw Go type name. func TestMapError_ErrorWithSuggestionSetsErrorType(t *testing.T) { tests := []struct { name string @@ -597,14 +804,28 @@ func TestMapError_ErrorWithSuggestionSetsErrorType(t *testing.T) { wantErrType string }{ { - name: "ResponseError_inner_type_recorded", + name: "Sentinel_uses_descriptive_code", + err: &internal.ErrorWithSuggestion{ + Err: fmt.Errorf( + "key not found: %w", + internal.ErrKeyNotFound), + Suggestion: "Run 'azd env get-values'.", + }, + wantErrCode: "error.suggestion", + wantErrType: "internal.key_not_found", + }, + { + name: "ResponseError_falls_back_to_go_type", err: &internal.ErrorWithSuggestion{ Err: &azcore.ResponseError{ ErrorCode: "QuotaExceeded", StatusCode: 429, RawResponse: &http.Response{ StatusCode: 429, - Request: &http.Request{Method: "POST", Host: "management.azure.com"}, + Request: &http.Request{ + Method: "POST", + Host: "management.azure.com", + }, }, }, Suggestion: "Request a quota increase.", @@ -613,7 +834,7 @@ func TestMapError_ErrorWithSuggestionSetsErrorType(t *testing.T) { wantErrType: "*exported.ResponseError", }, { - name: "PlainError_inner_type_recorded", + name: "PlainError_falls_back_to_go_type", err: &internal.ErrorWithSuggestion{ Err: errors.New("unknown failure"), Suggestion: "Try again.",