Merge pull request #2741 from ActiveState/DX-2151

User-facing errors in example runner

internal/errs/userfacing.go

@@ -0,0 +1,58 @@
+package errs
+
+import "errors"
+
+type UserFacingError interface {
+	error
+	UserError() string
+}
+
+type ErrOpt func(err *userFacingError) *userFacingError
+
+type userFacingError struct {
+	wrapped error
+	message string
+	tips    []string
+}
+
+func (e *userFacingError) Error() string {
+	return "User Facing Error: " + e.UserError()
+}
+
+func (e *userFacingError) UserError() string {
+	return e.message
+}
+
+func (e *userFacingError) ErrorTips() []string {
+	return e.tips
+}
+
+func NewUserFacingError(message string, tips ...string) *userFacingError {
+	return WrapUserFacingError(nil, message)
+}
+
+func WrapUserFacingError(wrapTarget error, message string, opts ...ErrOpt) *userFacingError {
+	err := &userFacingError{
+		wrapTarget,
+		message,
+		nil,
+	}
+
+	for _, opt := range opts {
+		err = opt(err)
+	}
+
+	return err
+}
+
+func IsUserFacing(err error) bool {
+	var userFacingError UserFacingError
+	return errors.As(err, &userFacingError)
+}
+
+func WithTips(tips ...string) ErrOpt {
+	return func(err *userFacingError) *userFacingError {
+		err.tips = append(err.tips, tips...)
+		return err
+	}
+}

internal/locale/errors.go

@@ -28,7 +28,7 @@ func (e *LocalizedError) Error() string {
 }
 
 // UserError is the user facing error message, it's the same as Error() but identifies it as being user facing
-func (e *LocalizedError) UserError() string {
+func (e *LocalizedError) LocalizedError() string {
 	return e.localized
 }
 
@@ -58,7 +58,7 @@ func (e *LocalizedError) AddTips(tips ...string) {
 // ErrorLocalizer represents a localized error
 type ErrorLocalizer interface {
 	error
-	UserError() string
+	LocalizedError() string
 }
 
 type AsError interface {
@@ -162,7 +162,7 @@ func JoinedErrorMessage(err error) string {
 	var message []string
 	for _, err := range UnpackError(err) {
 		if lerr, isLocaleError := err.(ErrorLocalizer); isLocaleError {
-			message = append(message, lerr.UserError())
+			message = append(message, lerr.LocalizedError())
 		}
 	}
 	if len(message) == 0 {
@@ -177,7 +177,7 @@ func JoinedErrorMessage(err error) string {
 
 func ErrorMessage(err error) string {
 	if errr, ok := err.(ErrorLocalizer); ok {
-		return errr.UserError()
+		return errr.LocalizedError()
 	}
 	return err.Error()
 }

internal/osutils/user/user.go

@@ -23,7 +23,7 @@ func (e *HomeDirNotFoundError) Error() string {
 	return homeDirNotFoundErrorMessage
 }
 
-func (e *HomeDirNotFoundError) UserError() string {
+func (e *HomeDirNotFoundError) LocalizedError() string {
 	return homeDirNotFoundErrorMessage
 }
 

internal/runbits/errors/centralized.go

@@ -0,0 +1,7 @@
+package errors
+
+import "github.com/ActiveState/cli/internal/errs"
+
+type ErrNoProject struct {
+	*errs.WrapperError
+}

internal/runbits/hello_example.go

@@ -1,14 +1,19 @@
 package runbits
 
 import (
+	"github.com/ActiveState/cli/internal/errs"
 	"github.com/ActiveState/cli/internal/locale"
 	"github.com/ActiveState/cli/internal/output"
 )
 
+type NoNameProvidedError struct {
+	*errs.WrapperError
+}
+
 func SayHello(out output.Outputer, name string) error {
 	if name == "" {
 		// Errors that are due to USER input should use `NewInputError` or `WrapInputError`
-		return locale.NewInputError("hello_err_no_name", "No name provided.")
+		return &NoNameProvidedError{errs.New("No name provided.")}
 	}
 
 	out.Print(locale.Tl("hello_message", "Hello, {{.V0}}!", name))

internal/runners/hello/hello_example.go

@@ -13,6 +13,7 @@ import (
 	"github.com/ActiveState/cli/internal/output"
 	"github.com/ActiveState/cli/internal/primer"
 	"github.com/ActiveState/cli/internal/runbits"
+	"github.com/ActiveState/cli/internal/runbits/errors"
 	"github.com/ActiveState/cli/pkg/platform/model"
 	"github.com/ActiveState/cli/pkg/project"
 )
@@ -37,9 +38,7 @@ type RunParams struct {
 // can be set. If no default or construction-time values are necessary, direct
 // construction of RunParams is fine, and this construction func may be dropped.
 func NewRunParams() *RunParams {
-	return &RunParams{
-		Name: "Friend",
-	}
+	return &RunParams{}
 }
 
 // Hello defines the app-level dependencies that are accessible within the Run
@@ -58,29 +57,49 @@ func New(p primeable) *Hello {
 	}
 }
 
+// processError contains the scope in which errors are processed. This is
+// useful for ensuring that errors are wrapped in a user-facing error and
+// localized.
+func processError(err *error) {
+	if err == nil {
+		return
+	}
+
+	switch {
+	case errs.Matches(*err, &runbits.NoNameProvidedError{}):
+		// Errors that we are looking for should be wrapped in a user-facing error.
+		// Ensure we wrap the top-level error returned from the runner and not
+		// the unpacked error that we are inspecting.
+		*err = errs.WrapUserFacingError(*err, locale.Tl("hello_err_no_name", "Cannot say hello because no name was provided."))
+	case errs.Matches(*err, &errors.ErrNoProject{}):
+		// It's useful to offer users reasonable tips on recourses.
+		*err = errs.WrapUserFacingError(
+			*err,
+			locale.Tl("hello_err_no_project", "Cannot say hello because you are not in a project directory."),
+			errs.WithTips(
+				locale.Tl("hello_suggest_checkout", "Try using [ACTIONABLE]`state checkout`[/RESET] first."),
+			),
+		)
+	}
+}
+
 // Run contains the scope in which the hello runner logic is executed.
-func (h *Hello) Run(params *RunParams) error {
+func (h *Hello) Run(params *RunParams) (rerr error) {
+	defer processError(&rerr)
+
 	h.out.Print(locale.Tl("hello_notice", "This command is for example use only"))
 
 	if h.project == nil {
-		err := locale.NewInputError(
-			"hello_info_err_no_project", "Not in a project directory.",
-		)
-
-		// It's useful to offer users reasonable tips on recourses.
-		return errs.AddTips(err, locale.Tl(
-			"hello_suggest_checkout",
-			"Try using [ACTIONABLE]`state checkout`[/RESET] first.",
-		))
+		return &errors.ErrNoProject{errs.New("Not in a project directory")}
 	}
 
 	// Reusable runner logic is contained within the runbits package.
 	// You should only use this if you intend to share logic between
 	// runners. Runners should NEVER invoke other runners.
 	if err := runbits.SayHello(h.out, params.Name); err != nil {
 		// Errors should nearly always be localized.
-		return locale.WrapError(
-			err, "hello_cannot_say", "Cannot say hello.",
+		return errs.Wrap(
+			err, "Cannot say hello.",
 		)
 	}
 
@@ -98,8 +117,8 @@ func (h *Hello) Run(params *RunParams) error {
 	// Grab data from the platform.
 	commitMsg, err := currentCommitMessage(h.project)
 	if err != nil {
-		err = locale.WrapError(
-			err, "hello_info_err_get_commit_msg", " Cannot get commit message",
+		err = errs.Wrap(
+			err, "Cannot get commit message",
 		)
 		return errs.AddTips(
 			err,
@@ -127,9 +146,7 @@ func currentCommitMessage(proj *project.Project) (string, error) {
 
 	commit, err := model.GetCommit(proj.CommitUUID())
 	if err != nil {
-		return "", locale.NewError(
-			"hello_info_err_get_commitr", "Cannot get commit from server",
-		)
+		return "", errs.Wrap(err, "Cannot get commit from server")
 	}
 
 	commitMsg := locale.Tl("hello_info_warn_no_commit", "Commit description not provided.")

pkg/cmdlets/errors/errors.go

@@ -39,19 +39,29 @@ func (o *OutputError) MarshalOutput(f output.Format) interface{} {
 		outLines = append(outLines, output.Title(locale.Tl("err_what_happened", "[ERROR]Something Went Wrong[/RESET]")).String())
 	}
 
-	rerrs := locale.UnpackError(o.error)
-	if len(rerrs) == 0 {
-		// It's possible the error came from cobra or something else low level that doesn't use localization
-		logging.Warning("Error does not have localization: %s", errs.JoinMessage(o.error))
-		rerrs = []error{o.error}
-	}
-	for _, errv := range rerrs {
-		message := trimError(locale.ErrorMessage(errv))
+	var userFacingError errs.UserFacingError
+	if errors.As(o.error, &userFacingError) {
+		message := userFacingError.UserError()
 		if f == output.PlainFormatName {
 			outLines = append(outLines, fmt.Sprintf(" [NOTICE][ERROR]x[/RESET] %s", message))
 		} else {
 			outLines = append(outLines, message)
 		}
+	} else {
+		rerrs := locale.UnpackError(o.error)
+		if len(rerrs) == 0 {
+			// It's possible the error came from cobra or something else low level that doesn't use localization
+			logging.Warning("Error does not have localization: %s", errs.JoinMessage(o.error))
+			rerrs = []error{o.error}
+		}
+		for _, errv := range rerrs {
+			message := trimError(locale.ErrorMessage(errv))
+			if f == output.PlainFormatName {
+				outLines = append(outLines, fmt.Sprintf(" [NOTICE][ERROR]x[/RESET] %s", message))
+			} else {
+				outLines = append(outLines, message)
+			}
+		}
 	}
 
 	// Concatenate error tips
@@ -103,6 +113,16 @@ func ParseUserFacing(err error) (int, error) {
 		return code, nil
 	}
 
+	// If there is a user facing error in the error stack we want to ensure
+	// that is it forwarded to the user.
+	var userFacingError errs.UserFacingError
+	if errors.As(err, &userFacingError) {
+		logging.Debug("Returning user facing error, error stack: \n%s", errs.JoinMessage(err))
+		return code, &OutputError{userFacingError}
+	}
+
+	// If the error already has a marshalling function we do not want to wrap
+	// it again in the OutputError type.
 	if hasMarshaller {
 		return code, err
 	}
@@ -157,7 +177,7 @@ func ReportError(err error, cmd *captain.Command, an analytics.Dispatcher) {
 		Error: ptr.To(errorMsg),
 	})
 
-	if !locale.HasError(err) && isErrs && !hasMarshaller {
+	if (!locale.HasError(err) && !errs.IsUserFacing(err)) && isErrs && !hasMarshaller {
 		multilog.Error("MUST ADDRESS: Error does not have localization: %s", errs.JoinMessage(err))
 
 		// If this wasn't built via CI then this is a dev workstation, and we should be more aggressive

pkg/platform/api/buildplanner/model/buildplan.go

@@ -107,7 +107,7 @@ func (e *BuildPlannerError) InputError() bool {
 // UserError returns the error message to be displayed to the user.
 // This function is added so that BuildPlannerErrors will be displayed
 // to the user
-func (e *BuildPlannerError) UserError() string {
+func (e *BuildPlannerError) LocalizedError() string {
 	return e.Error()
 }
 

pkg/platform/runtime/setup/setup.go

@@ -96,7 +96,7 @@ func (a *ArtifactSetupErrors) Errors() []error {
 }
 
 // UserError returns a message including all user-facing sub-error messages
-func (a *ArtifactSetupErrors) UserError() string {
+func (a *ArtifactSetupErrors) LocalizedError() string {
 	var errStrings []string
 	for _, err := range a.errs {
 		errStrings = append(errStrings, locale.JoinedErrorMessage(err))