Unify host gRPC error transport with a structured ActionableErrorDetail (#7919)

* Unify host gRPC error transport with structured ActionableErrorDetail

Replace the ad-hoc "<err>\n<suggestion>" concatenation in the host gRPC
error path with a structured proto detail, and consolidate the helpers
that serialize/deserialize host errors so suggestion and links travel as
typed data instead of being parsed out of the message string.

- Add ActionableErrorDetail { suggestion, links } proto (host -> extension)
- Rename wrapErrorWithSuggestion -> mapHostError; statusMessage no longer
  concatenates the suggestion into status.Message
- Remove duplicated grpcStatusFromError / wrapErrorLinks helpers in
  internal/grpcserver in favor of exported azdext counterparts
- Factor WrapError through populateExtensionErrorFromStatus; drop the
  extErr.Message == err.Error() heuristic
- Extend ErrorSuggestion / ErrorMessage / ErrorLinks to read host-attached
  ActionableErrorDetail so links survive the wire

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Address feedback

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: JeffreyCA

cli/azd/grpc/proto/errors.proto

@@ -36,8 +36,25 @@ message ErrorLink {
   string title = 2;  // Display text (falls back to the URL when empty)
 }
 
-// ExtensionError is a unified error message that can represent errors from different sources.
-// It provides structured error information for telemetry and error handling.
+// ActionableErrorDetail carries user-facing remediation metadata in gRPC status details.
+// It is used for host-originated errors so clients can preserve suggestions and links
+// without parsing status message text.
+//
+// Direction: host -> extension only. Extensions returning errors to the host should use
+// LocalError / ServiceError (transported via ExtensionError) instead.
+//
+// The user-facing error message lives in google.rpc.Status.message; this detail does not
+// duplicate it.
+message ActionableErrorDetail {
+  string suggestion = 1;         // Optional user-facing suggestion for resolving the error
+  repeated ErrorLink links = 2;  // Optional reference links rendered alongside the suggestion
+}
+
+// ExtensionError is a unified error message that can represent errors from different sources.
+// It provides structured error information for telemetry and error handling.
+//
+// Direction: extension -> host only. Counterpart of host's ActionableErrorDetail, which carries
+// host-originated remediation metadata in the opposite direction.
 message ExtensionError {
   reserved 1, 3;            // Reserved for future use; do not reuse.
   string message = 2;       // Human-readable error message

cli/azd/internal/grpcserver/errors.go

@@ -0,0 +1,144 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package grpcserver
+
+import (
+	"errors"
+	"fmt"
+	"log"
+
+	"github.com/azure/azure-dev/cli/azd/internal"
+	"github.com/azure/azure-dev/cli/azd/pkg/auth"
+	"github.com/azure/azure-dev/cli/azd/pkg/azdext"
+	"google.golang.org/genproto/googleapis/rpc/errdetails"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+// mapHostError serializes a host-originated Go error into a gRPC status error for transport
+// to extensions.
+//
+// This is the gRPC serialization boundary: the original error chain (e.g. *auth.AuthFailedError)
+// is intentionally not preserved across the wire — only the structured details defined here.
+//
+// Auth-related errors are reported with codes.Unauthenticated and an azd.auth ErrorInfo detail
+// (preserving AADSTS<code> reasons from Entra). ErrorWithSuggestion errors carry an
+// ActionableErrorDetail so consumers receive suggestion + links structurally.
+//
+// status.Message is always err.Error() (or ErrorWithSuggestion.Message when set). Suggestion
+// text is never concatenated into status.Message; consumers must read ActionableErrorDetail
+// for remediation guidance.
+func mapHostError(err error) error {
+	if err == nil {
+		return nil
+	}
+
+	suggestionErr, hasSuggestion := errors.AsType[*internal.ErrorWithSuggestion](err)
+	isAuthErr := isAuthError(err)
+	if !hasSuggestion && !isAuthErr {
+		return err
+	}
+
+	code := codes.Unknown
+	if st, ok := azdext.GRPCStatusFromError(err); ok {
+		code = st.Code()
+	}
+	if isAuthErr {
+		code = codes.Unauthenticated
+	}
+
+	st := status.New(code, statusMessage(err, suggestionErr))
+	if isAuthErr {
+		st = withAuthErrorInfo(st, err)
+	}
+	if hasSuggestion {
+		st = withActionableErrorDetail(st, suggestionErr)
+	}
+
+	return st.Err()
+}
+
+// statusMessage returns the user-facing message that should populate status.Message.
+// When the source is an ErrorWithSuggestion with an explicit Message, that wins. Otherwise,
+// if err is already a gRPC status error, use its Message (avoids nesting "rpc error: ..."
+// prefixes in the new status). Falls back to err.Error(). Suggestion text is never appended.
+func statusMessage(err error, suggestionErr *internal.ErrorWithSuggestion) string {
+	if suggestionErr != nil && suggestionErr.Message != "" {
+		return suggestionErr.Message
+	}
+	if st, ok := azdext.GRPCStatusFromError(err); ok {
+		return st.Message()
+	}
+	return err.Error()
+}
+
+// isAuthError reports whether err's chain contains a known auth-failure type that should be
+// surfaced over gRPC as codes.Unauthenticated.
+func isAuthError(err error) bool {
+	if errors.Is(err, auth.ErrNoCurrentUser) {
+		return true
+	}
+	if _, ok := errors.AsType[*auth.ReLoginRequiredError](err); ok {
+		return true
+	}
+	if _, ok := errors.AsType[*auth.AuthFailedError](err); ok {
+		return true
+	}
+	return false
+}
+
+func withAuthErrorInfo(st *status.Status, err error) *status.Status {
+	reason := grpcAuthReason(err)
+	if reason == "" {
+		return st
+	}
+
+	withDetails, detailErr := st.WithDetails(&errdetails.ErrorInfo{
+		Reason: reason,
+		Domain: azdext.AuthErrorDomain,
+	})
+	if detailErr != nil {
+		log.Printf("failed to attach auth ErrorInfo to gRPC status: %v", detailErr)
+		return st
+	}
+
+	return withDetails
+}
+
+func withActionableErrorDetail(st *status.Status, err *internal.ErrorWithSuggestion) *status.Status {
+	if err.Suggestion == "" && len(err.Links) == 0 {
+		return st
+	}
+
+	withDetails, detailErr := st.WithDetails(&azdext.ActionableErrorDetail{
+		Suggestion: err.Suggestion,
+		Links:      azdext.WrapErrorLinks(err.Links),
+	})
+	if detailErr != nil {
+		log.Printf("failed to attach ActionableErrorDetail to gRPC status: %v", detailErr)
+		return st
+	}
+
+	return withDetails
+}
+
+func grpcAuthReason(err error) string {
+	if errors.Is(err, auth.ErrNoCurrentUser) {
+		return azdext.AuthErrorReasonNotLoggedIn
+	}
+
+	// Pass through the originating AAD error code (e.g., "AADSTS530084") when available.
+	// This preserves Entra's own semantics rather than redefining them on azd's side.
+	if authFailed, ok := errors.AsType[*auth.AuthFailedError](err); ok {
+		if authFailed.Parsed != nil && len(authFailed.Parsed.ErrorCodes) > 0 {
+			return fmt.Sprintf("AADSTS%d", authFailed.Parsed.ErrorCodes[0])
+		}
+	}
+
+	if _, ok := errors.AsType[*auth.ReLoginRequiredError](err); ok {
+		return azdext.AuthErrorReasonLoginRequired
+	}
+
+	return ""
+}

cli/azd/internal/grpcserver/prompt_service_test.go

@@ -665,9 +665,15 @@ func Test_PromptService_PromptSubscription_ErrorWithSuggestion(t *testing.T) {
 	})
 
 	require.Error(t, err)
-	// Verify that the suggestion text is included in the error message (wrapped by interceptor)
-	require.Contains(t, err.Error(), "azd auth login")
+	// Status message carries only the underlying error; suggestion travels via ActionableErrorDetail.
 	require.Contains(t, err.Error(), "AADSTS70043")
+	require.NotContains(t, err.Error(), "azd auth login",
+		"suggestion text must not be concatenated into status.Message")
+	st, ok := status.FromError(err)
+	require.True(t, ok)
+	actionable := azdext.ActionableErrorDetailFromStatus(st)
+	require.NotNil(t, actionable)
+	require.Contains(t, actionable.GetSuggestion(), "azd auth login")
 	mockPrompter.AssertExpectations(t)
 }
 
@@ -697,9 +703,15 @@ func Test_PromptService_PromptResourceGroup_ErrorWithSuggestion(t *testing.T) {
 	})
 
 	require.Error(t, err)
-	// Verify that the suggestion text is included in the error message (wrapped by interceptor)
-	require.Contains(t, err.Error(), "azd auth login")
+	// Status message carries only the underlying error; suggestion travels via ActionableErrorDetail.
 	require.Contains(t, err.Error(), "AADSTS70043")
+	require.NotContains(t, err.Error(), "azd auth login",
+		"suggestion text must not be concatenated into status.Message")
+	st, ok := status.FromError(err)
+	require.True(t, ok)
+	actionable := azdext.ActionableErrorDetailFromStatus(st)
+	require.NotNil(t, actionable)
+	require.Contains(t, actionable.GetSuggestion(), "azd auth login")
 	mockPrompter.AssertExpectations(t)
 }
 

cli/azd/internal/grpcserver/server.go

@@ -6,16 +6,12 @@ package grpcserver
 import (
 	"context"
 	"crypto/rand"
-	"errors"
 	"fmt"
 	"log"
 	"net"
 
-	"github.com/azure/azure-dev/cli/azd/internal"
-	"github.com/azure/azure-dev/cli/azd/pkg/auth"
 	"github.com/azure/azure-dev/cli/azd/pkg/azdext"
 	"github.com/azure/azure-dev/cli/azd/pkg/extensions"
-	"google.golang.org/genproto/googleapis/rpc/errdetails"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/metadata"
@@ -159,9 +155,9 @@ func (s *Server) Stop() error {
 	return nil
 }
 
-// errorWrappingInterceptor wraps ErrorWithSuggestion errors to include their suggestion text
-// in the error message. This ensures that helpful suggestions (like "run azd auth login")
-// are preserved when errors are transmitted over gRPC, where only the error message string is sent.
+// errorWrappingInterceptor maps host errors into gRPC status errors with structured details
+// (auth ErrorInfo, ActionableErrorDetail) so extensions can preserve actionable guidance
+// without parsing status message text. See [mapHostError] for the contract.
 func (s *Server) errorWrappingInterceptor() grpc.UnaryServerInterceptor {
 	return func(
 		ctx context.Context,
@@ -171,15 +167,13 @@ func (s *Server) errorWrappingInterceptor() grpc.UnaryServerInterceptor {
 	) (any, error) {
 		resp, err := handler(ctx, req)
 		if err != nil {
-			err = wrapErrorWithSuggestion(err)
+			err = mapHostError(err)
 		}
 		return resp, err
 	}
 }
 
 // errorWrappingStreamInterceptor is the streaming counterpart of errorWrappingInterceptor.
-// It wraps ErrorWithSuggestion errors from stream handlers so that actionable suggestions
-// are preserved in gRPC stream error responses.
 func (s *Server) errorWrappingStreamInterceptor() grpc.StreamServerInterceptor {
 	return func(
 		srv any,
@@ -189,7 +183,7 @@ func (s *Server) errorWrappingStreamInterceptor() grpc.StreamServerInterceptor {
 	) error {
 		err := handler(srv, ss)
 		if err != nil {
-			err = wrapErrorWithSuggestion(err)
+			err = mapHostError(err)
 		}
 		return err
 	}
@@ -268,89 +262,6 @@ func (s *authenticatedStream) Context() context.Context {
 	return s.ctx
 }
 
-// wrapErrorWithSuggestion checks if the error contains an ErrorWithSuggestion and if so,
-// returns a new error that includes the suggestion text in the error message.
-// This ensures that helpful suggestions (like "run azd auth login") are preserved
-// when errors are transmitted over gRPC, where only the error message string is sent.
-//
-// Auth-related errors are returned with codes.Unauthenticated and a structured
-// ErrorInfo reason so extensions can reliably classify them as auth failures
-// while still distinguishing the remediation path.
-func wrapErrorWithSuggestion(err error) error {
-	if err == nil {
-		return nil
-	}
-
-	isAuthErr := isAuthError(err)
-
-	if suggestionErr, ok := errors.AsType[*internal.ErrorWithSuggestion](err); ok {
-		msg := fmt.Sprintf("%s\n%s", err.Error(), suggestionErr.Suggestion)
-		if isAuthErr {
-			return grpcAuthStatus(err, msg)
-		}
-		return fmt.Errorf("%w\n%s", err, suggestionErr.Suggestion)
-	}
-
-	if isAuthErr {
-		return grpcAuthStatus(err, err.Error())
-	}
-
-	return err
-}
-
-// isAuthError reports whether err's chain contains a known auth-failure type that should be
-// surfaced over gRPC as codes.Unauthenticated.
-func isAuthError(err error) bool {
-	if errors.Is(err, auth.ErrNoCurrentUser) {
-		return true
-	}
-	if _, ok := errors.AsType[*auth.ReLoginRequiredError](err); ok {
-		return true
-	}
-	if _, ok := errors.AsType[*auth.AuthFailedError](err); ok {
-		return true
-	}
-	return false
-}
-
-func grpcAuthStatus(err error, msg string) error {
-	st := status.New(codes.Unauthenticated, msg)
-	reason := grpcAuthReason(err)
-	if reason == "" {
-		return st.Err()
-	}
-
-	withDetails, detailErr := st.WithDetails(&errdetails.ErrorInfo{
-		Reason: reason,
-		Domain: azdext.AuthErrorDomain,
-	})
-	if detailErr != nil {
-		return st.Err()
-	}
-
-	return withDetails.Err()
-}
-
-func grpcAuthReason(err error) string {
-	if errors.Is(err, auth.ErrNoCurrentUser) {
-		return azdext.AuthErrorReasonNotLoggedIn
-	}
-
-	// Pass through the originating AAD error code (e.g., "AADSTS530084") when available.
-	// This preserves Entra's own semantics rather than redefining them on azd's side.
-	if authFailed, ok := errors.AsType[*auth.AuthFailedError](err); ok {
-		if authFailed.Parsed != nil && len(authFailed.Parsed.ErrorCodes) > 0 {
-			return fmt.Sprintf("AADSTS%d", authFailed.Parsed.ErrorCodes[0])
-		}
-	}
-
-	if _, ok := errors.AsType[*auth.ReLoginRequiredError](err); ok {
-		return azdext.AuthErrorReasonLoginRequired
-	}
-
-	return ""
-}
-
 func generateSigningKey() ([]byte, error) {
 	bytes := make([]byte, 32) // 256-bit HMAC signing key
 	if _, err := rand.Read(bytes); err != nil {