From 2cadb80fb2b6b0d6ef9c9259b5b77c62c7e9d2d0 Mon Sep 17 00:00:00 2001
From: Percy Wegmann <percy@tailscale.com>
Date: Wed, 9 Oct 2024 12:05:33 -0500
Subject: [PATCH] util/vizerror: add WrapWithMessage

Thus new function allows constructing vizerrors that combine a message
appropriate for display to users with a wrapped underlying error.

Updates tailscale/corp#23781

Signed-off-by: Percy Wegmann <percy@tailscale.com>
---
 util/vizerror/vizerror.go      | 58 ++++++++++++++++++++++++++--------
 util/vizerror/vizerror_test.go | 22 +++++++++++++
 2 files changed, 67 insertions(+), 13 deletions(-)

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)
+	}
+}