Introduce new-style redactable errors.

This change equips the error library with string redaction according
to the algorithms and ideas from github.com/cockroachdb/redact.

It is backward incompatible in two ways:

- it breaks support with Go 1.12. The code is now 1.13+ only.
- the `errors.Safe()` function has a new return type.

New:

- When an error object is passed as formatting argument to `Newf()`,
  `Wrapf()` and similar (i.e. not as first argument in `Wrap`),
  it is now attached as secondary payload as per
  `errors.WithSecondaryError()`.
  This ensures that more details about these errors are included
  eventually in Sentry reports.

- The full text of the error message, with sensitive bits redacted
  out, is now included at the head of Sentry reports.

- The new `errors.SafeFormatter` interface mimics `errors.Formatter`,
  with a method `SafeFormatError()`. Its `Printer` argument has
  `Print()` methods that behave like `redact.SafeWriter`: they
  distinguish sensitive and non-sensitive error bits.

  For example:

  ```go
  func (myError) SafeFormatError(p errors.Printer) {
     p.Printf("hello %s", "world")
     //        ^^^^^ safe
     //                   ^^^^^^^ unsafe

     p.Printf("hello %s", errors.Safe("world"))
     //        ^^^^^ safe
     //                   ^^^^^^^^^^^^^^^^^^^^^ safe
  }
  ```

  This method can be implemented *instead of* `FormatError()`; the
  error library will know how to use it if present.

- The new `errors.Formattable()` wrapper transforms any `error`
  instance into a `fmt.Formatter` which will detail the error
  adequately, even if the error type does not implement `FormatError`.
  Example use:

  ```go
  fmt.Printf("hello: %v", errors.Formattable(err))
  ```

API changes:

- `errors.Formatter` continues to be supported for compatibility
  with Go 1.13's `xerror` proposal. It is however deprecated
  in favor of `errors.SafeFormatter`.

- `errors.Safe()` is now an alias for `redact.Safe()`. It now returns
  a `redact.SafeValue` instead of a `SafeMessager`.

- The `errors.SafeMessager` interface is deprecated. Use
  the `redact.SafeFormatter` interface, or the other
  APIs from the `redact` package.

- The "complex" constructors `errors.New*()`, `errors.Wrap*()` now use
  facilities from the `redact` package to distinguish safe and unsafe
  arguments, instead of the `WithSafeDetails()` wrapper. This makes
  the chain of errors simpler.

Author: knz

.travis.yml

@@ -1,6 +1,5 @@
 language: go
 
 go:
-- 1.12.x
 - 1.13.x
 - 1.14.x

README.md

@@ -46,6 +46,7 @@ Table of contents:
 | wrappers to attach secondary causes                                                                   |                     |                         |                            | ✔                    |
 | wrappers to attach [`logtags`](https://github.com/cockroachdb/logtags) details from `context.Context` |                     |                         |                            | ✔                    |
 | `errors.FormatError()`, `Formatter`, `Printer`                                                        |                     |                         | (under construction)       | ✔                    |
+| `errors.SafeFormatError()`, `SafeFormatter`                                                           |                     |                         |                            | ✔                    |
 
 "Forward compatibility" above refers to the ability of this library to
 recognize and properly handle network communication of error types it
@@ -201,17 +202,17 @@ return errors.Wrap(foo(), "foo")
 - `WithMessage(error, string) error`, `WithMessagef(error, string, ...interface{}) error`: message prefix.
   - when to use: probably never. Use `errors.Wrap()`/`errors.Wrapf()` instead.
   - what it does: adds a message prefix.
-  - how to access the detail: `Error()`, regular Go formatting. Not included in Sentry reports.
+  - how to access the detail: `Error()`, regular Go formatting, Sentry Report.
 
 - `WithDetail(error, string) error`, `WithDetailf(error, string, ...interface{}) error`, user-facing detail with contextual information.
   - **when to use: need to embark a message string to output when the error is presented to a human.**
   - what it does: captures detail strings.
-  - how to access the detail: `errors.GetAllDetails()`, `errors.FlattenDetails()` (all details are preserved), format with `%+v`.
+  - how to access the detail: `errors.GetAllDetails()`, `errors.FlattenDetails()` (all details are preserved), format with `%+v`. Not included in Sentry reports.
 
 - `WithHint(error, string) error`, `WithHintf(error, string, ...interface{}) error`: user-facing detail with suggestion for action to take.
   - **when to use: need to embark a message string to output when the error is presented to a human.**
   - what it does: captures hint strings.
-  - how to access the detail: `errors.GetAllHints()`, `errors.FlattenHints()` (hints are de-duplicated), format with `%+v`.
+  - how to access the detail: `errors.GetAllHints()`, `errors.FlattenHints()` (hints are de-duplicated), format with `%+v`. Not included in Sentry reports.
 
 - `WithIssueLink(error, IssueLink) error`: annotate an error with an URL and arbitrary string.
   - **when to use: to refer (human) users to some external resources.**
@@ -260,7 +261,7 @@ considered to be PII-free, and thus included in Sentry reports automatically:
 - the `format string` argument of `Newf`, `AssertionFailedf`, etc (the constructors ending with `...f()`),
 - the *type* of the additional arguments passed to the `...f()` constructors,
 - the *value of specific argument types* passed to the `...f()` constructors, when known to be PII-safe.
-  For details of which arguments are considered PII-free, see the [`Redact()` function](safedetails/redact.go).
+  For details of which arguments are considered PII-free, see the [`redact` package](https://github.com/cockroachdb/redact).
 
 It is possible to opt additional in to Sentry reporting, using either of the following methods:
 
@@ -293,7 +294,7 @@ If your error type is a wrapper, you should implement a `Format()`
 method that redirects to `errors.FormatError()`, otherwise `%+v` will
 not work. Additionally, if your type has a payload not otherwise
 visible via `Error()`, you may want to implement
-`errors.Formatter`. See [making `%+v` work with your
+`errors.SafeFormatter`. See [making `%+v` work with your
 type](#Making-v-work-with-your-type) below for details.
 
 Finally, you may want your new error type to be portable across
@@ -445,8 +446,8 @@ In short:
   type, this will disable the recursive application of the `%+v` flag
   to the causes chained from your wrapper.)
 
-- You may optionally implement the `errors.Formatter` interface:
-  `FormatError(p errors.Printer) (next error)`.  This is optional, but
+- You may optionally implement the `errors.SafeFormatter` interface:
+  `SafeFormatError(p errors.Printer) (next error)`.  This is optional, but
   should be done when some details are not included by `Error()` and
   should be emitted upon `%+v`.
 
@@ -458,11 +459,11 @@ achieves this as follows:
 func (w *withHTTPCode) Format(s fmt.State, verb rune) { errors.FormatError(w, s, verb) }
 
 // FormatError() formats the error.
-func (w *withHTTPCode) FormatError(p errors.Printer) (next error) {
+func (w *withHTTPCode) SafeFormatError(p errors.Printer) (next error) {
 	// Note: no need to print out the cause here!
 	// FormatError() knows how to do this automatically.
 	if p.Detail() {
-		p.Printf("http code: %d", w.code)
+		p.Printf("http code: %d", errors.Safe(w.code))
 	}
 	return w.cause
 }
@@ -492,22 +493,22 @@ proposal](https://go.googlesource.com/proposal/+/master/design/29934-error-value
 
 ## Error composition (summary)
 
-| Constructor                        | Composes                                                                                         |
-|------------------------------------|--------------------------------------------------------------------------------------------------|
-| `New`                              | `NewWithDepth` (see below)                                                                       |
-| `Errorf`                           | = `Newf`                                                                                         |
-| `Newf`                             | `NewWithDepthf` (see below)                                                                      |
-| `WithMessage`                      | = `pkgErr.WithMessage`                                                                           |
-| `Wrap`                             | `WrapWithDepth` (see below)                                                                      |
-| `Wrapf`                            | `WrapWithDepthf` (see below)                                                                     |
-| `AssertionFailed`                  | `AssertionFailedWithDepthf` (see below)                                                          |
-| `NewWithDepth`                     | `goErr.New` + `WithStackDepth` (see below)                                                       |
-| `NewWithDepthf`                    | `fmt.Errorf` + `WithSafeDetails` + `WithStackDepth`                                              |
-| `WithMessagef`                     | `pkgErr.WithMessagef` + `WithSafeDetails`                                                        |
-| `WrapWithDepth`                    | `WithMessage` + `WithStackDepth`                                                                 |
-| `WrapWithDepthf`                   | `WithMessage` + `WithStackDepth` + `WithSafeDetails`                                             |
-| `AssertionFailedWithDepthf`        | `fmt.Errorf` + `WithStackDepth` + `WithSafeDetails` + `WithAssertionFailure`                     |
-| `NewAssertionErrorWithWrappedErrf` | `HandledWithMessagef` (barrier) + `WithStackDepth` + `WithSafeDetails` +  `WithAssertionFailure` |
+| Constructor                        | Composes                                                                          |
+|------------------------------------|-----------------------------------------------------------------------------------|
+| `New`                              | `NewWithDepth` (see below)                                                        |
+| `Errorf`                           | = `Newf`                                                                          |
+| `Newf`                             | `NewWithDepthf` (see below)                                                       |
+| `WithMessage`                      | custom wrapper with message prefix and knowledge of safe strings                  |
+| `Wrap`                             | `WrapWithDepth` (see below)                                                       |
+| `Wrapf`                            | `WrapWithDepthf` (see below)                                                      |
+| `AssertionFailed`                  | `AssertionFailedWithDepthf` (see below)                                           |
+| `NewWithDepth`                     | custom leaf with knowledge of safe strings + `WithStackDepth` (see below)         |
+| `NewWithDepthf`                    | custom leaf with knowledge of safe strings + `WithSafeDetails` + `WithStackDepth` |
+| `WithMessagef`                     | custom wrapper with message prefix and knowledge of safe strings                  |
+| `WrapWithDepth`                    | `WithMessage` + `WithStackDepth`                                                  |
+| `WrapWithDepthf`                   | `WithMessagef` + `WithStackDepth`                                                 |
+| `AssertionFailedWithDepthf`        | `NewWithDepthf` + `WithAssertionFailure`                                          |
+| `NewAssertionErrorWithWrappedErrf` | `HandledWithMessagef` (barrier) + `WrapWithDepthf` +  `WithAssertionFailure`      |
 
 ## API (not constructing error objects)
 
@@ -519,6 +520,13 @@ func Cause(err error) error // compatibility
 func Unwrap(err error) error // compatibility
 type Wrapper interface { ... } // compatibility
 
+// Error formatting.
+type Formatter interface { ... } // compatibility, not recommended
+type SafeFormatter interface { ... }
+type Printer interface { ... }
+func FormatError(err error, s fmt.State, verb rune)
+func Formattable(err error) fmt.Formatter
+
 // Identify errors.
 func Is(err, reference error) bool
 func IsAny(err error, references ...error) bool
@@ -553,10 +561,14 @@ func GetReportableStackTrace(err error) *ReportableStackTrace
 type SafeDetailPayload struct { ... }
 func GetAllSafeDetails(err error) []SafeDetailPayload
 func GetSafeDetails(err error) (payload SafeDetailPayload)
+
+// Obsolete APIs.
 type SafeMessager interface { ... }
-func Safe(v interface{}) SafeMessager
 func Redact(r interface{}) string
 
+// Aliases redact.Safe.
+func Safe(v interface{}) SafeMessager
+
 // Assertion failures.
 func HasAssertionFailure(err error) bool
 func IsAssertionFailure(err error) bool

assert/assert.go

@@ -62,7 +62,7 @@ type withAssertionFailure struct {
 
 var _ error = (*withAssertionFailure)(nil)
 var _ fmt.Formatter = (*withAssertionFailure)(nil)
-var _ errbase.Formatter = (*withAssertionFailure)(nil)
+var _ errbase.SafeFormatter = (*withAssertionFailure)(nil)
 
 // ErrorHint implements the hintdetail.ErrorHinter interface.
 func (w *withAssertionFailure) ErrorHint() string {
@@ -77,9 +77,9 @@ func (w *withAssertionFailure) Cause() error  { return w.cause }
 func (w *withAssertionFailure) Unwrap() error { return w.cause }
 
 func (w *withAssertionFailure) Format(s fmt.State, verb rune) { errbase.FormatError(w, s, verb) }
-func (w *withAssertionFailure) FormatError(p errbase.Printer) error {
+func (w *withAssertionFailure) SafeFormatError(p errbase.Printer) error {
 	if p.Detail() {
-		p.Print("assertion failure")
+		p.Printf("assertion failure")
 	}
 	return w.cause
 }

barriers/barriers.go

@@ -75,7 +75,7 @@ type barrierError struct {
 
 var _ error = (*barrierError)(nil)
 var _ errbase.SafeDetailer = (*barrierError)(nil)
-var _ errbase.Formatter = (*barrierError)(nil)
+var _ errbase.SafeFormatter = (*barrierError)(nil)
 var _ fmt.Formatter = (*barrierError)(nil)
 
 // barrierError is an error.
@@ -94,8 +94,8 @@ func (e *barrierError) SafeDetails() []string {
 // Printing a barrier reveals the details.
 func (e *barrierError) Format(s fmt.State, verb rune) { errbase.FormatError(e, s, verb) }
 
-func (e *barrierError) FormatError(p errbase.Printer) (next error) {
-	p.Printf(e.msg)
+func (e *barrierError) SafeFormatError(p errbase.Printer) (next error) {
+	p.Print(e.msg)
 	if p.Detail() {
 		p.Printf("-- cause hidden behind barrier\n%+v", e.maskedErr)
 	}

contexttags/contexttags.go

@@ -18,8 +18,8 @@ import (
 	"context"
 
 	"github.com/cockroachdb/errors/errbase"
-	"github.com/cockroachdb/errors/safedetails"
 	"github.com/cockroachdb/logtags"
+	"github.com/cockroachdb/redact"
 )
 
 // WithContextTags captures the k/v pairs stored in the context via the
@@ -88,15 +88,26 @@ func convertToStringsOnly(b *logtags.Buffer) (res *logtags.Buffer) {
 
 func redactTags(b *logtags.Buffer) []string {
 	res := make([]string, len(b.Get()))
+	redactableTagsIterate(b, func(i int, r redact.RedactableString) {
+		res[i] = r.Redact().StripMarkers()
+	})
+	return res
+}
+
+func redactableTagsIterate(b *logtags.Buffer, fn func(i int, s redact.RedactableString)) {
+	var empty redact.SafeString
 	for i, t := range b.Get() {
-		res[i] = t.Key()
+		k := t.Key()
 		v := t.Value()
+		eq := empty
+		var val interface{} = empty
 		if v != nil {
-			if len(res[i]) > 1 {
-				res[i] += "="
+			if len(k) > 1 {
+				eq = "="
 			}
-			res[i] += safedetails.Redact(v)
+			val = v
 		}
+		res := redact.Sprintf("%s%s%v", redact.Safe(k), eq, val)
+		fn(i, res)
 	}
-	return res
 }

contexttags/contexttags_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/cockroachdb/errors/markers"
 	"github.com/cockroachdb/errors/testutils"
 	"github.com/cockroachdb/logtags"
+	"github.com/cockroachdb/redact"
 )
 
 func TestWithContext(t *testing.T) {
@@ -87,7 +88,7 @@ func TestWithContext(t *testing.T) {
 	tt.Run("remote", func(tt testutils.T) { theTest(tt, newErr) })
 }
 
-func TestTagRedaction(t *testing.T) {
+func TestTagRedactionInSafeDetails(t *testing.T) {
 	tt := testutils.T{T: t}
 
 	// Create an example context with decoration.
@@ -105,10 +106,13 @@ func TestTagRedaction(t *testing.T) {
 	ctx2 = logtags.AddTag(ctx2, "planet1", "universe")
 	ctx2 = logtags.AddTag(ctx2, "planet2", errors.Safe("universe"))
 
-	// This will be our reference expected value.
+	// rm is what's left over after redaction.
+	rm := string(redact.RedactableBytes(redact.RedactedMarker()).StripMarkers())
+
+	// This will be our reference expected value in "safe details" strings.
 	refStrings := [][]string{
-		[]string{"planet1=string:<redacted>", "planet2=universe"},
-		[]string{"foo1=int:<redacted>", "xint:<redacted>", "bar1", "foo2=123", "y456", "bar2"},
+		[]string{"planet1=" + rm, "planet2=universe"},
+		[]string{"foo1=" + rm, "x" + rm, "bar1", "foo2=123", "y456", "bar2"},
 	}
 
 	// Construct the error object.

contexttags/with_context.go

@@ -21,6 +21,7 @@ import (
 	"github.com/cockroachdb/errors/errbase"
 	"github.com/cockroachdb/errors/errorspb"
 	"github.com/cockroachdb/logtags"
+	"github.com/cockroachdb/redact"
 	"github.com/gogo/protobuf/proto"
 )
 
@@ -39,8 +40,8 @@ type withContext struct {
 
 var _ error = (*withContext)(nil)
 var _ errbase.SafeDetailer = (*withContext)(nil)
+var _ errbase.SafeFormatter = (*withContext)(nil)
 var _ fmt.Formatter = (*withContext)(nil)
-var _ errbase.Formatter = (*withContext)(nil)
 
 // withContext is an error. The original error message is preserved.
 func (w *withContext) Error() string { return w.cause.Error() }
@@ -52,9 +53,16 @@ func (w *withContext) Unwrap() error { return w.cause }
 // Printing a withContext reveals the tags.
 func (w *withContext) Format(s fmt.State, verb rune) { errbase.FormatError(w, s, verb) }
 
-func (w *withContext) FormatError(p errbase.Printer) error {
+func (w *withContext) SafeFormatError(p errbase.Printer) error {
 	if p.Detail() && w.tags != nil {
-		p.Printf("tags: [%s]", w.tags.String())
+		p.Printf("tags: [")
+		redactableTagsIterate(w.tags, func(i int, r redact.RedactableString) {
+			if i > 0 {
+				p.Printf(",")
+			}
+			p.Print(r)
+		})
+		p.Printf("]")
 	}
 	return w.cause
 }

domains/domains.go

@@ -93,7 +93,7 @@ func HandledInDomainWithMessage(err error, domain Domain, msg string) error {
 // Handled creates a handled error in the implicit domain (see
 // PackageDomain() below) of its caller.
 //
-// See the documentation of `errors.Handled()` for details.
+// See the documentation of `barriers.Handled()` for details.
 func Handled(err error) error {
 	return HandledInDomain(err, PackageDomainAtDepth(1))
 }

domains/with_domain.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 @@ var _ error = (*withDomain)(nil)
 var _ errbase.SafeDetailer = (*withDomain)(nil)
 var _ errbase.TypeKeyMarker = (*withDomain)(nil)
 var _ fmt.Formatter = (*withDomain)(nil)
-var _ errbase.Formatter = (*withDomain)(nil)
+var _ errbase.SafeFormatter = (*withDomain)(nil)
 
 // withDomain is an error. The original error message is preserved.
 func (e *withDomain) Error() string { return e.cause.Error() }
@@ -58,9 +59,9 @@ func (e *withDomain) SafeDetails() []string {
 
 func (e *withDomain) Format(s fmt.State, verb rune) { errbase.FormatError(e, s, verb) }
 
-func (e *withDomain) FormatError(p errbase.Printer) error {
+func (e *withDomain) SafeFormatError(p errbase.Printer) error {
 	if p.Detail() {
-		p.Print(e.domain)
+		p.Print(redact.Safe(e.domain))
 	}
 	return e.cause
 }

errbase/encode.go

@@ -211,6 +211,9 @@ func getTypeDetails(
 
 // TypeKeyMarker can be implemented by errors that wish to extend
 // their type name as seen by GetTypeKey().
+//
+// Note: the key marker is considered safe for reporting and
+// is included in sentry reports.
 type TypeKeyMarker interface {
 	ErrorKeyMarker() string
 }