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
5 changes: 5 additions & 0 deletions internal/errs/centralized.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package errs

type ErrNoProject struct {
*WrapperError
}
MDrakos marked this conversation as resolved.
Show resolved Hide resolved
54 changes: 54 additions & 0 deletions internal/errs/errs.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,33 @@ type TransientError interface {
IsTransient()
}

type UserFacingError interface {
MDrakos marked this conversation as resolved.
Show resolved Hide resolved
error
UserError() string
}

type userFacingError struct {
wrapped error
message string
tips []string
}

func (e *userFacingError) Error() string {
return JoinMessage(Wrap(e.wrapped, e.message))
MDrakos marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return JoinMessage(Wrap(e.wrapped, e.message))
return "User Facing Error: " + e.UserError()

}

func (e *userFacingError) UserError() string {
MDrakos marked this conversation as resolved.
Show resolved Hide resolved
return e.message
}

func (e *userFacingError) AddTips(tips ...string) {
e.tips = append(e.tips, tips...)
}

func (e *userFacingError) ErrorTips() []string {
return e.tips
}

// PackedErrors represents a collection of errors that aren't necessarily related to each other
// note that rtutils replicates this functionality to avoid import cycles
type PackedErrors struct {
Expand Down Expand Up @@ -104,6 +131,22 @@ func Wrap(wrapTarget error, message string, args ...interface{}) *WrapperError {
return newError(msg, wrapTarget)
}

func NewUserFacingError(message string, args ...interface{}) *userFacingError {
return &userFacingError{
nil,
fmt.Sprintf(message, args...),
nil,
}
MDrakos marked this conversation as resolved.
Show resolved Hide resolved
}

func WrapUserFacingError(wrapTarget error, message string, args ...interface{}) *userFacingError {
MDrakos marked this conversation as resolved.
Show resolved Hide resolved
return &userFacingError{
wrapTarget,
fmt.Sprintf(message, args...),
nil,
}
}

// Pack creates a new error that packs the given errors together, allowing for multiple errors to be returned
func Pack(err error, errs ...error) error {
return &PackedErrors{append([]error{err}, errs...)}
Expand Down Expand Up @@ -258,3 +301,14 @@ func Unpack(err error) []error {
}
return result
}

// IsUserFacing identifies whether this error was curated for end-users and
// returns that error. This is NOT the same as an error that is localized.
func IsUserFacing(err error) (error, bool) {
var userFacingError UserFacingError
if errors.As(err, &userFacingError) {
return userFacingError, true
}

return err, false
MDrakos marked this conversation as resolved.
Show resolved Hide resolved
}
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
55 changes: 35 additions & 20 deletions internal/runners/hello/hello_example.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,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 +56,48 @@ 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 {
MDrakos marked this conversation as resolved.
Show resolved Hide resolved
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, &errs.ErrNoProject{}):
werr := errs.WrapUserFacingError(*err, locale.Tl("hello_err_no_project", "Cannot say hello because you are not in a project directory."))

// It's useful to offer users reasonable tips on recourses.
*err = errs.AddTips(werr, locale.Tl(
"hello_suggest_checkout",
"Try using [ACTIONABLE]`state checkout`[/RESET] first.",
))
MDrakos marked this conversation as resolved.
Show resolved Hide resolved
}

*err = errs.Wrap(*err, "Cannot say hello")
MDrakos marked this conversation as resolved.
Show resolved Hide resolved
}
}

// 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 &errs.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 +115,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 +144,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
10 changes: 10 additions & 0 deletions pkg/cmdlets/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,21 @@ func ParseUserFacing(err error) (int, error) {
// unwrap exit code before we remove un-localized wrapped errors from err variable
code := errs.ParseExitCode(err)

// If there is a user facing error in the error stack we want to ensure
// that is it forwarded to the user.
uerr, ok := errs.IsUserFacing(err)
if ok {
logging.Debug("Returning user facing error, error stack: \n%s", errs.JoinMessage(err))
return code, &OutputError{uerr}
Copy link
Member

Choose a reason for hiding this comment

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

I don't think OutputError produces the error string from UserFacingError()?

Please make sure you run this code to ensure it produces the error message you expect.

Copy link
Member Author

Choose a reason for hiding this comment

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

It does here: https://github.com/ActiveState/cli/blob/master/pkg/cmdlets/errors/errors.go. I've confirmed this from running the code.

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand, you linked to code on master which definitely would not be handling this new concept. Wrong link?

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, yes wrong link.

I've run the code and confirmed the output from UserError() is there. I believe it's done via the output mediator here: https://github.com/ActiveState/cli/blob/DX-2151/internal/output/mediator.go#L54

}

if errs.IsSilent(err) {
logging.Debug("Suppressing silent failure: %v", err.Error())
return code, nil
}

// 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
Loading