diff --git a/internal/errs/userfacing.go b/internal/errs/userfacing.go new file mode 100644 index 0000000000..5ae0df22dc --- /dev/null +++ b/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 + } +} diff --git a/internal/locale/errors.go b/internal/locale/errors.go index ae3460afe6..61cde512e5 100644 --- a/internal/locale/errors.go +++ b/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() } diff --git a/internal/osutils/user/user.go b/internal/osutils/user/user.go index 44b51d7c4f..56bacb3791 100644 --- a/internal/osutils/user/user.go +++ b/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 } diff --git a/internal/runbits/errors/centralized.go b/internal/runbits/errors/centralized.go new file mode 100644 index 0000000000..3f005a1fda --- /dev/null +++ b/internal/runbits/errors/centralized.go @@ -0,0 +1,7 @@ +package errors + +import "github.com/ActiveState/cli/internal/errs" + +type ErrNoProject struct { + *errs.WrapperError +} diff --git a/internal/runbits/hello_example.go b/internal/runbits/hello_example.go index 93c0c65c32..53e852d31f 100644 --- a/internal/runbits/hello_example.go +++ b/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)) diff --git a/internal/runners/hello/hello_example.go b/internal/runners/hello/hello_example.go index d336a88640..41f43a8064 100644 --- a/internal/runners/hello/hello_example.go +++ b/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,20 +57,40 @@ 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. @@ -79,8 +98,8 @@ func (h *Hello) Run(params *RunParams) error { // 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.") diff --git a/pkg/cmdlets/errors/errors.go b/pkg/cmdlets/errors/errors.go index a47b2747c0..7c2691de79 100644 --- a/pkg/cmdlets/errors/errors.go +++ b/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 diff --git a/pkg/platform/api/buildplanner/model/buildplan.go b/pkg/platform/api/buildplanner/model/buildplan.go index e24117a660..bb4376ba79 100644 --- a/pkg/platform/api/buildplanner/model/buildplan.go +++ b/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() } diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index b48f0cfdb7..accba52564 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/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))