support wrapError encode and decode

Previously, support for the `wrapError` type generated by
`fmt.Errorf("%w xxxx")` was missing an understanding of the fact
that the format string wasn't limited to just prefixes. Hence the
`opaqueWrapper` struct assumed that all wrappers had prefix strings
that would precede their causal chains.

This change adds a flag to the `EncodedWrapper` which tracks whether
the wrapper "owns" the error string. In the case of `fmt.Errorf`
the `wrapError` struct created within the stdlib contains a fully
interpolated string as its message, not the format string, or a
prefix. This means that when we encode it into a `EncodedWrapper`
for transfer over the network, we need to remember to just print the
wrapper's string when `.Error()` is called on the decoded object on
the other end.

Paired with this change the `opaqueWrapper` that is generated on the
decode side now contains this field as well which is referenced when
calling `.Error()` on the wrapper, ensuring we don't re-concatenate
the causes. This field is also referenced when formatting the error
in `state.formatSingleLineOutput` where we use it to simply output
the final entry's message, instead of concatenating all the entries
together.

This change is backwards compatible in that prior versions will not
set the flag on the protobuf, and hence will continue to encode mis-
configured `wrapError` instances that will see messages repeated when
converted to Strings.

Author: dhartunian

contexttags/with_context.go

@@ -75,13 +75,13 @@ func (w *withContext) SafeDetails() []string {
 	return redactTags(w.tags)
 }
 
-func encodeWithContext(_ context.Context, err error) (string, []string, proto.Message) {
+func encodeWithContext(_ context.Context, err error) (string, []string, proto.Message, bool) {
 	w := err.(*withContext)
 	p := &errorspb.TagsPayload{}
 	for _, t := range w.tags.Get() {
 		p.Tags = append(p.Tags, errorspb.TagPayload{Tag: t.Key(), Value: t.ValueStr()})
 	}
-	return "", w.SafeDetails(), p
+	return "", w.SafeDetails(), p, false
 }
 
 func decodeWithContext(

errbase/adapters.go

@@ -76,21 +76,21 @@ func decodeWithMessage(
 
 func encodePkgWithStack(
 	_ context.Context, err error,
-) (msgPrefix string, safe []string, _ proto.Message) {
+) (msgPrefix string, safe []string, _ proto.Message, _ bool) {
 	iErr := err.(interface{ StackTrace() pkgErr.StackTrace })
 	safeDetails := []string{fmt.Sprintf("%+v", iErr.StackTrace())}
-	return "" /* withStack does not have a message prefix */, safeDetails, nil
+	return "" /* withStack does not have a message prefix */, safeDetails, nil, false
 }
 
 func encodePathError(
 	_ context.Context, err error,
-) (msgPrefix string, safe []string, details proto.Message) {
+) (msgPrefix string, safe []string, details proto.Message, ownError bool) {
 	p := err.(*os.PathError)
 	msg := p.Op + " " + p.Path
 	details = &errorspb.StringsPayload{
 		Details: []string{p.Op, p.Path},
 	}
-	return msg, []string{p.Op}, details
+	return msg, []string{p.Op}, details, false
 }
 
 func decodePathError(
@@ -113,13 +113,13 @@ func decodePathError(
 
 func encodeLinkError(
 	_ context.Context, err error,
-) (msgPrefix string, safe []string, details proto.Message) {
+) (msgPrefix string, safe []string, details proto.Message, ownError bool) {
 	p := err.(*os.LinkError)
 	msg := p.Op + " " + p.Old + " " + p.New
 	details = &errorspb.StringsPayload{
 		Details: []string{p.Op, p.Old, p.New},
 	}
-	return msg, []string{p.Op}, details
+	return msg, []string{p.Op}, details, false
 }
 
 func decodeLinkError(
@@ -143,9 +143,9 @@ func decodeLinkError(
 
 func encodeSyscallError(
 	_ context.Context, err error,
-) (msgPrefix string, safe []string, details proto.Message) {
+) (msgPrefix string, safe []string, details proto.Message, ownError bool) {
 	p := err.(*os.SyscallError)
-	return p.Syscall, nil, nil
+	return p.Syscall, nil, nil, false
 }
 
 func decodeSyscallError(
@@ -211,4 +211,12 @@ func init() {
 	RegisterWrapperDecoder(pKey, decodeSyscallError)
 
 	RegisterLeafEncoder(GetTypeKey(&OpaqueErrno{}), encodeOpaqueErrno)
+
+	RegisterWrapperEncoder(GetTypeKey(fmt.Errorf("text %w text", baseErr)), encodeWrapError)
+}
+
+func encodeWrapError(_ context.Context, err error) (
+	msgPrefix string, safeDetails []string, payload proto.Message, ownError bool,
+) {
+	return err.Error(), nil, nil, true
 }

errbase/adapters_test.go

@@ -55,6 +55,18 @@ func TestAdaptBaseGoErr(t *testing.T) {
 	tt.CheckDeepEqual(newErr, origErr)
 }
 
+func TestAdaptGoSingleWrapErr(t *testing.T) {
+	origErr := fmt.Errorf("an error %w", goErr.New("hello"))
+	t.Logf("start err: %# v", pretty.Formatter(origErr))
+
+	newErr := network(t, origErr)
+
+	tt := testutils.T{T: t}
+	// The library preserves the cause. It's not possible to preserve the fmt
+	// string.
+	tt.CheckContains(newErr.Error(), "hello")
+}
+
 func TestAdaptPkgWithMessage(t *testing.T) {
 	// Simple message wrappers from github.com/pkg/errors are preserved
 	// completely.

errbase/decode.go

@@ -107,9 +107,10 @@ func decodeWrapper(ctx context.Context, enc *errorspb.EncodedWrapper) error {
 
 	// Otherwise, preserve all details about the original object.
 	return &opaqueWrapper{
-		cause:   cause,
-		prefix:  enc.MessagePrefix,
-		details: enc.Details,
+		cause:           cause,
+		prefix:          enc.MessagePrefix,
+		details:         enc.Details,
+		ownsErrorString: enc.WrapperOwnsErrorString,
 	}
 }
 

errbase/encode.go

@@ -115,6 +115,7 @@ func encodeAsAny(ctx context.Context, err error, payload proto.Message) *types.A
 func encodeWrapper(ctx context.Context, err, cause error) EncodedError {
 	var msg string
 	var details errorspb.EncodedErrorDetails
+	var ownError bool
 
 	if e, ok := err.(*opaqueWrapper); ok {
 		msg = e.prefix
@@ -127,7 +128,7 @@ func encodeWrapper(ctx context.Context, err, cause error) EncodedError {
 		// If we have a manually registered encoder, use that.
 		typeKey := TypeKey(details.ErrorTypeMark.FamilyName)
 		if enc, ok := encoders[typeKey]; ok {
-			msg, details.ReportablePayload, payload = enc(ctx, err)
+			msg, details.ReportablePayload, payload, ownError = enc(ctx, err)
 		} else {
 			// No encoder.
 			// In that case, we'll try to compute a message prefix
@@ -148,19 +149,22 @@ func encodeWrapper(ctx context.Context, err, cause error) EncodedError {
 	return EncodedError{
 		Error: &errorspb.EncodedError_Wrapper{
 			Wrapper: &errorspb.EncodedWrapper{
-				Cause:         EncodeError(ctx, cause),
-				MessagePrefix: msg,
-				Details:       details,
+				Cause:                  EncodeError(ctx, cause),
+				MessagePrefix:          msg,
+				Details:                details,
+				WrapperOwnsErrorString: ownError,
 			},
 		},
 	}
 }
 
 // extractPrefix extracts the prefix from a wrapper's error message.
 // For example,
-//    err := errors.New("bar")
-//    err = errors.Wrap(err, "foo")
-//    extractPrefix(err)
+//
+//	err := errors.New("bar")
+//	err = errors.Wrap(err, "foo")
+//	extractPrefix(err)
+//
 // returns "foo".
 func extractPrefix(err, cause error) string {
 	causeSuffix := cause.Error()
@@ -304,7 +308,7 @@ func RegisterWrapperEncoder(theType TypeKey, encoder WrapperEncoder) {
 
 // WrapperEncoder is to be provided (via RegisterWrapperEncoder above)
 // by additional wrapper types not yet known to this library.
-type WrapperEncoder = func(ctx context.Context, err error) (msgPrefix string, safeDetails []string, payload proto.Message)
+type WrapperEncoder = func(ctx context.Context, err error) (msgPrefix string, safeDetails []string, payload proto.Message, ownError bool)
 
 // registry for RegisterWrapperType.
 var encoders = map[TypeKey]WrapperEncoder{}

errbase/format_error.go

@@ -147,7 +147,11 @@ func formatErrorInternal(err error, s fmt.State, verb rune, redactableOutput boo
 		// recursion. So we have no choice but to peel the data
 		// and then assemble the pieces ourselves.
 		p.formatRecursive(err, true /* isOutermost */, false /* withDetail */)
-		p.formatSingleLineOutput()
+		if ee, ok := err.(*opaqueWrapper); ok {
+			p.formatSingleLineOutput(ee.ownsErrorString)
+		} else {
+			p.formatSingleLineOutput(false)
+		}
 		p.finishDisplay(verb)
 
 	default:
@@ -185,7 +189,7 @@ func (s *state) formatEntries(err error) {
 	//
 	//   <complete error message>
 	//   (1) <details>
-	s.formatSingleLineOutput()
+	s.formatSingleLineOutput(false)
 	s.finalBuf.WriteString("\n(1)")
 
 	s.printEntry(s.entries[len(s.entries)-1])
@@ -278,8 +282,9 @@ func (s *state) printEntry(entry formatEntry) {
 //
 // This function is used both when FormatError() is called indirectly
 // from .Error(), e.g. in:
-//      (e *myType) Error() { return fmt.Sprintf("%v", e) }
-//      (e *myType) Format(s fmt.State, verb rune) { errors.FormatError(s, verb, e) }
+//
+//	(e *myType) Error() { return fmt.Sprintf("%v", e) }
+//	(e *myType) Format(s fmt.State, verb rune) { errors.FormatError(s, verb, e) }
 //
 // and also to print the first line in the output of a %+v format.
 //
@@ -290,7 +295,7 @@ func (s *state) printEntry(entry formatEntry) {
 // RedactableBytes. However, we are not using the helper facilities
 // from redact.SafePrinter to do this, so care should be taken below
 // to properly escape markers, etc.
-func (s *state) formatSingleLineOutput() {
+func (s *state) formatSingleLineOutput(parentOwnsErrorString bool) {
 	for i := len(s.entries) - 1; i >= 0; i-- {
 		entry := &s.entries[i]
 		if entry.elideShort {
@@ -316,6 +321,10 @@ func (s *state) formatSingleLineOutput() {
 			// This means entry.head is unsafe. We need to escape it.
 			s.finalBuf.Write([]byte(redact.EscapeBytes(entry.head)))
 		}
+		if parentOwnsErrorString {
+			// only process the final wrapper
+			return
+		}
 	}
 }
 

errbase/format_error_internal_test.go

@@ -65,7 +65,7 @@ func TestFormatSingleLineOutput(t *testing.T) {
 
 	for _, tc := range testCases {
 		s := state{entries: tc.entries}
-		s.formatSingleLineOutput()
+		s.formatSingleLineOutput(false)
 		if s.finalBuf.String() != tc.exp {
 			t.Errorf("%s: expected %q, got %q", tc.entries, tc.exp, s.finalBuf.String())
 		}
@@ -165,7 +165,7 @@ func TestFormatSingleLineOutputRedactable(t *testing.T) {
 
 	for _, tc := range testCases {
 		s := state{entries: tc.entries, redactableOutput: true}
-		s.formatSingleLineOutput()
+		s.formatSingleLineOutput(false)
 		if s.finalBuf.String() != tc.exp {
 			t.Errorf("%s: expected %q, got %q", tc.entries, tc.exp, s.finalBuf.String())
 		}

errbase/opaque.go

@@ -40,9 +40,10 @@ var _ SafeFormatter = (*opaqueLeaf)(nil)
 // back to some network system that _does_ know about
 // the type, the original object can be restored.
 type opaqueWrapper struct {
-	cause   error
-	prefix  string
-	details errorspb.EncodedErrorDetails
+	cause           error
+	prefix          string
+	details         errorspb.EncodedErrorDetails
+	ownsErrorString bool
 }
 
 var _ error = (*opaqueWrapper)(nil)
@@ -53,6 +54,9 @@ var _ SafeFormatter = (*opaqueWrapper)(nil)
 func (e *opaqueLeaf) Error() string { return e.msg }
 
 func (e *opaqueWrapper) Error() string {
+	if e.ownsErrorString {
+		return e.prefix
+	}
 	if e.prefix == "" {
 		return e.cause.Error()
 	}

errbase/unknown_type_test.go

@@ -162,9 +162,9 @@ func TestUnknownWrapperTraversal(t *testing.T) {
 	t.Logf("start err: %# v", pretty.Formatter(origErr))
 
 	// Register a temporary encoder.
-	myEncode := func(_ context.Context, err error) (string, []string, proto.Message) {
+	myEncode := func(_ context.Context, err error) (string, []string, proto.Message, bool) {
 		m := err.(*myWrap)
-		return "", nil, &internal.MyPayload{Val: int32(m.val)}
+		return "", nil, &internal.MyPayload{Val: int32(m.val)}, false
 	}
 	tn := errbase.GetTypeKey((*myWrap)(nil))
 	errbase.RegisterWrapperEncoder(tn, myEncode)