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.

Sign up for GitHub

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

Improve the structure of sentry reports #94

Merged
merged 3 commits into from
Mar 9, 2022
Merged

Improve the structure of sentry reports #94

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d46ef1
barriers: reveal more safe details about the masked error
knz Mar 4, 2022
a73e1b6
report: rework the composition to use redacted verbose printouts
knz Mar 4, 2022
feb9d32
barriers: use a redactable string as message payload
knz Mar 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 47 additions & 20 deletions barriers/barriers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"

"github.com/cockroachdb/errors/errbase"
"github.com/cockroachdb/redact"
"github.com/gogo/protobuf/proto"
)

Expand All @@ -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.
Expand All @@ -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
Expand All @@ -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)
}
Expand All @@ -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)
}
14 changes: 7 additions & 7 deletions barriers/barriers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions errutil/message_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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`,
},
}

Expand Down
Loading