Merge pull request #94 from knz/20220304-barrier-details

Author: knz

barriers/barriers.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 
 	"github.com/cockroachdb/errors/errbase"
+	"github.com/cockroachdb/redact"
 	"github.com/gogo/protobuf/proto"
 )
 
@@ -36,7 +37,7 @@ func Handled(err error) error {
 	if err == nil {
 		return nil
 	}
-	return HandledWithMessage(err, err.Error())
+	return HandledWithSafeMessage(err, redact.Sprint(err))
 }
 
 // HandledWithMessage is like Handled except the message is overridden.
@@ -46,7 +47,14 @@ func HandledWithMessage(err error, msg string) error {
 	if err == nil {
 		return nil
 	}
-	return &barrierError{maskedErr: err, msg: msg}
+	return HandledWithSafeMessage(err, redact.Sprint(msg))
+}
+
+// HandledWithSafeMessage is like Handled except the message is overridden.
+// This can be used e.g. to hide message details or to prevent
+// downstream code to make assertions on the message's contents.
+func HandledWithSafeMessage(err error, msg redact.RedactableString) error {
+	return &barrierErr{maskedErr: err, smsg: msg}
 }
 
 // HandledWithMessagef is like HandledWithMessagef except the message
@@ -55,47 +63,48 @@ func HandledWithMessagef(err error, format string, args ...interface{}) error {
 	if err == nil {
 		return nil
 	}
-	return &barrierError{maskedErr: err, msg: fmt.Sprintf(format, args...)}
+	return &barrierErr{maskedErr: err, smsg: redact.Sprintf(format, args...)}
 }
 
-// barrierError is a leaf error type. It encapsulates a chain of
+// barrierErr is a leaf error type. It encapsulates a chain of
 // original causes, but these causes are hidden so that they inhibit
 // matching via Is() and the Cause()/Unwrap() recursions.
-type barrierError struct {
+type barrierErr struct {
 	// Message for the barrier itself.
 	// In the common case, the message from the masked error
 	// is used as-is (see Handled() above) however it is
 	// useful to cache it here since the masked error may
 	// have a long chain of wrappers and its Error() call
 	// may be expensive.
-	msg string
+	smsg redact.RedactableString
 	// Masked error chain.
 	maskedErr error
 }
 
-var _ error = (*barrierError)(nil)
-var _ errbase.SafeDetailer = (*barrierError)(nil)
-var _ errbase.SafeFormatter = (*barrierError)(nil)
-var _ fmt.Formatter = (*barrierError)(nil)
+var _ error = (*barrierErr)(nil)
+var _ errbase.SafeDetailer = (*barrierErr)(nil)
+var _ errbase.SafeFormatter = (*barrierErr)(nil)
+var _ fmt.Formatter = (*barrierErr)(nil)
 
-// barrierError is an error.
-func (e *barrierError) Error() string { return e.msg }
+// barrierErr is an error.
+func (e *barrierErr) Error() string { return e.smsg.StripMarkers() }
 
 // SafeDetails reports the PII-free details from the masked error.
-func (e *barrierError) SafeDetails() []string {
+func (e *barrierErr) SafeDetails() []string {
 	var details []string
 	for err := e.maskedErr; err != nil; err = errbase.UnwrapOnce(err) {
 		sd := errbase.GetSafeDetails(err)
 		details = sd.Fill(details)
 	}
+	details = append(details, redact.Sprintf("masked error: %+v", e.maskedErr).Redact().StripMarkers())
 	return details
 }
 
 // Printing a barrier reveals the details.
-func (e *barrierError) Format(s fmt.State, verb rune) { errbase.FormatError(e, s, verb) }
+func (e *barrierErr) Format(s fmt.State, verb rune) { errbase.FormatError(e, s, verb) }
 
-func (e *barrierError) SafeFormatError(p errbase.Printer) (next error) {
-	p.Print(e.msg)
+func (e *barrierErr) SafeFormatError(p errbase.Printer) (next error) {
+	p.Print(e.smsg)
 	if p.Detail() {
 		p.Printf("-- cause hidden behind barrier\n%+v", e.maskedErr)
 	}
@@ -106,19 +115,37 @@ func (e *barrierError) SafeFormatError(p errbase.Printer) (next error) {
 func encodeBarrier(
 	ctx context.Context, err error,
 ) (msg string, details []string, payload proto.Message) {
-	e := err.(*barrierError)
+	e := err.(*barrierErr)
 	enc := errbase.EncodeError(ctx, e.maskedErr)
-	return e.msg, e.SafeDetails(), &enc
+	return string(e.smsg), e.SafeDetails(), &enc
 }
 
 // A barrier error is decoded exactly.
 func decodeBarrier(ctx context.Context, msg string, _ []string, payload proto.Message) error {
 	enc := payload.(*errbase.EncodedError)
-	return &barrierError{msg: msg, maskedErr: errbase.DecodeError(ctx, *enc)}
+	return &barrierErr{smsg: redact.RedactableString(msg), maskedErr: errbase.DecodeError(ctx, *enc)}
 }
 
+// Previous versions of barrier errors.
+func decodeBarrierPrev(ctx context.Context, msg string, _ []string, payload proto.Message) error {
+	enc := payload.(*errbase.EncodedError)
+	return &barrierErr{smsg: redact.Sprint(msg), maskedErr: errbase.DecodeError(ctx, *enc)}
+}
+
+// barrierError is the "old" type name of barrierErr. We use a new
+// name now to ensure a different decode function is used when
+// importing barriers from the previous structure, where the
+// message is not redactable.
+type barrierError struct {
+	msg       string
+	maskedErr error
+}
+
+func (b *barrierError) Error() string { return "" }
+
 func init() {
-	tn := errbase.GetTypeKey((*barrierError)(nil))
+	errbase.RegisterLeafDecoder(errbase.GetTypeKey((*barrierError)(nil)), decodeBarrierPrev)
+	tn := errbase.GetTypeKey((*barrierErr)(nil))
 	errbase.RegisterLeafDecoder(tn, decodeBarrier)
 	errbase.RegisterLeafEncoder(tn, encodeBarrier)
 }

barriers/barriers_test.go

@@ -118,7 +118,7 @@ woo
   | woo
   | (1) woo
   | Error types: (1) *errors.errorString
-Error types: (1) *barriers.barrierError`},
+Error types: (1) *barriers.barrierErr`},
 
 		{"handled + handled", barriers.Handled(barriers.Handled(goErr.New("woo"))), woo, `
 woo
@@ -130,8 +130,8 @@ woo
   |   | woo
   |   | (1) woo
   |   | Error types: (1) *errors.errorString
-  | Error types: (1) *barriers.barrierError
-Error types: (1) *barriers.barrierError`},
+  | Error types: (1) *barriers.barrierErr
+Error types: (1) *barriers.barrierErr`},
 
 		{"handledmsg", barriers.HandledWithMessage(goErr.New("woo"), "waa"), "waa", `
 waa
@@ -140,7 +140,7 @@ waa
   | woo
   | (1) woo
   | Error types: (1) *errors.errorString
-Error types: (1) *barriers.barrierError`},
+Error types: (1) *barriers.barrierErr`},
 
 		{"handledmsg + handledmsg", barriers.HandledWithMessage(
 			barriers.HandledWithMessage(
@@ -154,8 +154,8 @@ wuu
   |   | woo
   |   | (1) woo
   |   | Error types: (1) *errors.errorString
-  | Error types: (1) *barriers.barrierError
-Error types: (1) *barriers.barrierError`},
+  | Error types: (1) *barriers.barrierErr
+Error types: (1) *barriers.barrierErr`},
 
 		{"handled + wrapper",
 			barriers.Handled(
@@ -172,7 +172,7 @@ waa: woo
   |   | multi-line wrapper payload
   | Wraps: (2) woo
   | Error types: (1) *barriers_test.werrFmt (2) *errors.errorString
-Error types: (1) *barriers.barrierError`},
+Error types: (1) *barriers.barrierErr`},
 	}
 
 	for _, test := range testCases {

errutil/message_test.go

@@ -124,7 +124,7 @@ Wraps: (4) wuu: woo
   |   | multi-line payload
   | Wraps: (2) woo
   | Error types: (1) *errutil_test.werrFmt (2) *errors.errorString
-Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *errutil.withPrefix (4) *barriers.barrierError`,
+Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *errutil.withPrefix (4) *barriers.barrierErr`,
 		},
 
 		{"assert + wrap empty",
@@ -148,7 +148,7 @@ Wraps: (3) wuu: woo
   |   | multi-line payload
   | Wraps: (2) woo
   | Error types: (1) *errutil_test.werrFmt (2) *errors.errorString
-Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *barriers.barrierError`,
+Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *barriers.barrierErr`,
 		},
 
 		{"assert + wrap empty+arg",
@@ -173,7 +173,7 @@ Wraps: (4) wuu: woo
   |   | multi-line payload
   | Wraps: (2) woo
   | Error types: (1) *errutil_test.werrFmt (2) *errors.errorString
-Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *errutil.withPrefix (4) *barriers.barrierError`,
+Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *errutil.withPrefix (4) *barriers.barrierErr`,
 		},
 	}