diff --git a/util/vizerror/vizerror.go b/util/vizerror/vizerror.go index 158786494..919d765d0 100644 --- a/util/vizerror/vizerror.go +++ b/util/vizerror/vizerror.go @@ -12,35 +12,67 @@ // Error is an error that is safe to display to end users. type Error struct { - err error + publicErr error // visible to end users + wrapped error // internal } -// Error implements the error interface. +// Error implements the error interface. The returned string is safe to display +// to end users. func (e Error) Error() string { - return e.err.Error() + return e.publicErr.Error() } // New returns an error that formats as the given text. It always returns a vizerror.Error. -func New(text string) error { - return Error{errors.New(text)} +func New(publicMsg string) error { + err := errors.New(publicMsg) + return Error{ + publicErr: err, + wrapped: err, + } } -// Errorf returns an Error with the specified format and values. It always returns a vizerror.Error. -func Errorf(format string, a ...any) error { - return Error{fmt.Errorf(format, a...)} +// Errorf returns an Error with the specified publicMsgFormat and values. It always returns a vizerror.Error. +// +// Warning: avoid using an error as one of the format arguments, as this will cause the text +// of that error to be displayed to the end user (which is probably not what you want). +func Errorf(publicMsgFormat string, a ...any) error { + err := fmt.Errorf(publicMsgFormat, a...) + return Error{ + publicErr: err, + wrapped: err, + } } // Unwrap returns the underlying error. +// +// If the Error was constructed using [WrapWithMessage], this is the wrapped (internal) error +// and not the user-visible error message. func (e Error) Unwrap() error { - return e.err + return e.wrapped } -// Wrap wraps err with a vizerror.Error. -func Wrap(err error) error { - if err == nil { +// Wrap wraps publicErr with a vizerror.Error. +// +// Deprecated: this is almost always the wrong thing to do. Are you really sure +// you know exactly what err.Error() will stringify to and be safe to show to +// users? [WrapWithMessage] is probably what you want. +func Wrap(publicErr error) error { + if publicErr == nil { return nil } - return Error{err} + return Error{publicErr: publicErr, wrapped: publicErr} +} + +// WrapWithMessage wraps the given error with a message that's safe to display +// to end users. The text of the wrapped error will not be displayed to end +// users. +// +// WrapWithMessage should almost always be preferred to [Wrap]. +func WrapWithMessage(wrapped error, publicMsg string) error { + return Error{ + publicErr: errors.New(publicMsg), + wrapped: wrapped, + } } // As returns the first vizerror.Error in err's chain. diff --git a/util/vizerror/vizerror_test.go b/util/vizerror/vizerror_test.go index bbd2c07e5..242ca6462 100644 --- a/util/vizerror/vizerror_test.go +++ b/util/vizerror/vizerror_test.go @@ -42,3 +42,25 @@ func TestAs(t *testing.T) { t.Errorf("As() returned error %v, want %v", got, verr) } } + +func TestWrap(t *testing.T) { + wrapped := errors.New("wrapped") + err := Wrap(wrapped) + if err.Error() != "wrapped" { + t.Errorf(`Wrap(wrapped).Error() = %q, want %q`, err.Error(), "wrapped") + } + if errors.Unwrap(err) != wrapped { + t.Errorf("Unwrap = %q, want %q", errors.Unwrap(err), wrapped) + } +} + +func TestWrapWithMessage(t *testing.T) { + wrapped := errors.New("wrapped") + err := WrapWithMessage(wrapped, "safe") + if err.Error() != "safe" { + t.Errorf(`WrapWithMessage(wrapped, "safe").Error() = %q, want %q`, err.Error(), "safe") + } + if errors.Unwrap(err) != wrapped { + t.Errorf("Unwrap = %q, want %q", errors.Unwrap(err), wrapped) + } +}