Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User-facing errors in example runner #2741

Merged
merged 12 commits into from
Sep 15, 2023
58 changes: 58 additions & 0 deletions internal/errs/userfacing.go
Original file line number Diff line number Diff line change
@@ -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
}
}
8 changes: 4 additions & 4 deletions internal/locale/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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()
}
Expand Down
2 changes: 1 addition & 1 deletion internal/osutils/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func (e *HomeDirNotFoundError) Error() string {
return homeDirNotFoundErrorMessage
}

func (e *HomeDirNotFoundError) UserError() string {
func (e *HomeDirNotFoundError) LocalizedError() string {
return homeDirNotFoundErrorMessage
}

Expand Down
7 changes: 7 additions & 0 deletions internal/runbits/errors/centralized.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package errors

import "github.com/ActiveState/cli/internal/errs"

type ErrNoProject struct {
*errs.WrapperError
}
7 changes: 6 additions & 1 deletion internal/runbits/hello_example.go
Original file line number Diff line number Diff line change
@@ -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`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is now incorrect and causing confusion @Naatan @MDrakos

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))
Expand Down
57 changes: 37 additions & 20 deletions internal/runners/hello/hello_example.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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
Expand All @@ -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.",
)
}

Expand All @@ -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,
Expand Down Expand Up @@ -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.")
Expand Down
38 changes: 29 additions & 9 deletions pkg/cmdlets/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}
MDrakos marked this conversation as resolved.
Show resolved Hide resolved
}
MDrakos marked this conversation as resolved.
Show resolved Hide resolved

// 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
}
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/platform/api/buildplanner/model/buildplan.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,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()
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/platform/runtime/setup/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,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))
Expand Down
Loading