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 an enum to the `EncodedWrapper` which tracks whether
the wrapper's error string is a prefix, or the full error. 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 since older versions will omit the
new field in the proto, defaulting it to `PREFIX` in newer versions,
which will preserve existing behavior when printing out errors. New
code will set the field when necessary and interpret it appropriately.
If an error encoded with a new version is decoded by an older version,
the information about the error string will be discarded, which may
lead to repeated portions of the error message when printed out, if
the wrapper contains a copy of the message of its cause.

Author: dhartunian

README.md

@@ -573,9 +573,11 @@ func RegisterLeafDecoder(typeName TypeKey, decoder LeafDecoder)
 func RegisterLeafEncoder(typeName TypeKey, encoder LeafEncoder)
 func RegisterWrapperDecoder(typeName TypeKey, decoder WrapperDecoder)
 func RegisterWrapperEncoder(typeName TypeKey, encoder WrapperEncoder)
+func RegisterWrapperEncoderWithMessageOverride (typeName TypeKey, encoder WrapperEncoderWithMessageOverride)
 type LeafEncoder = func(ctx context.Context, err error) (msg string, safeDetails []string, payload proto.Message)
 type LeafDecoder = func(ctx context.Context, msg string, safeDetails []string, payload proto.Message) error
 type WrapperEncoder = func(ctx context.Context, err error) (msgPrefix string, safeDetails []string, payload proto.Message)
+type WrapperEncoderWithMessageOverride = func(ctx context.Context, err error) (msgPrefix string, safeDetails []string, payload proto.Message, overrideError bool)
 type WrapperDecoder = func(ctx context.Context, cause error, msgPrefix string, safeDetails []string, payload proto.Message) error
 
 // Registering package renames for custom error types.

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

@@ -97,7 +97,7 @@ func decodeWrapper(ctx context.Context, enc *errorspb.EncodedWrapper) error {
 	typeKey := TypeKey(enc.Details.ErrorTypeMark.FamilyName)
 	if decoder, ok := decoders[typeKey]; ok {
 		// Yes, use it.
-		genErr := decoder(ctx, cause, enc.MessagePrefix, enc.Details.ReportablePayload, payload)
+		genErr := decoder(ctx, cause, enc.Message, enc.Details.ReportablePayload, payload)
 		if genErr != nil {
 			// Decoding succeeded. Use this.
 			return genErr
@@ -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.Message,
+		details:     enc.Details,
+		messageType: MessageType(enc.MessageType),
 	}
 }
 

errbase/encode.go

@@ -115,10 +115,18 @@ 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
+	messageType := Prefix
 
 	if e, ok := err.(*opaqueWrapper); ok {
+		// We delegate all knowledge of the error string
+		// to the original encoder and do not try to re-engineer
+		// the prefix out of the error. This helps maintain
+		// backward compatibility with earlier versions of the
+		// encoder which don't have any understanding of
+		// error string ownership by the wrapper.
 		msg = e.prefix
 		details = e.details
+		messageType = e.messageType
 	} else {
 		details.OriginalTypeName, details.ErrorTypeMark.FamilyName, details.ErrorTypeMark.Extension = getTypeDetails(err, false /*onlyFamily*/)
 
@@ -127,12 +135,12 @@ 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, messageType = enc(ctx, err)
 		} else {
 			// No encoder.
 			// In that case, we'll try to compute a message prefix
 			// manually.
-			msg = extractPrefix(err, cause)
+			msg, messageType = extractPrefix(err, cause)
 
 			// If there are known safe details, use them.
 			if s, ok := err.(SafeDetailer); ok {
@@ -148,31 +156,47 @@ 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),
+				Message:     msg,
+				Details:     details,
+				MessageType: errorspb.MessageType(messageType),
 			},
 		},
 	}
 }
 
 // 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 {
+//
+// If a presumed wrapper does not have a message prefix, it is assumed
+// to override the entire error message and `extractPrefix` returns
+// the entire message and the boolean `true` to signify that the causes
+// should not be appended to it.
+func extractPrefix(err, cause error) (string, MessageType) {
 	causeSuffix := cause.Error()
 	errMsg := err.Error()
 
 	if strings.HasSuffix(errMsg, causeSuffix) {
 		prefix := errMsg[:len(errMsg)-len(causeSuffix)]
+		// If error msg matches exactly then this is a wrapper
+		// with no message of its own.
+		if len(prefix) == 0 {
+			return "", Prefix
+		}
 		if strings.HasSuffix(prefix, ": ") {
-			return prefix[:len(prefix)-2]
+			return prefix[:len(prefix)-2], Prefix
 		}
 	}
-	return ""
+	// If we don't have the cause as a suffix, then we have
+	// some other string as our error msg, preserve that and
+	// mark as override
+	return errMsg, FullMessage
 }
 
 func getTypeDetails(
@@ -295,6 +319,35 @@ var leafEncoders = map[TypeKey]LeafEncoder{}
 // or a different type, ensure that RegisterTypeMigration() was called
 // prior to RegisterWrapperEncoder().
 func RegisterWrapperEncoder(theType TypeKey, encoder WrapperEncoder) {
+	RegisterWrapperEncoderWithMessageType(
+		theType,
+		func(ctx context.Context, err error) (
+			msgPrefix string,
+			safeDetails []string,
+			payload proto.Message,
+			messageType MessageType,
+		) {
+			prefix, details, payload := encoder(ctx, err)
+			return prefix, details, payload, messageType
+		})
+}
+
+// RegisterWrapperEncoderWithMessageType can be used to register
+// new wrapper types to the library. Registered wrappers will be
+// encoded using their own Go type when an error is encoded. Wrappers
+// that have not been registered will be encoded using the
+// opaqueWrapper type.
+//
+// This function differs from RegisterWrapperEncoder by allowing the
+// caller to explicitly decide whether the wrapper owns the entire
+// error message or not. Otherwise, the relationship is inferred.
+//
+// Note: if the error type has been migrated from a previous location
+// or a different type, ensure that RegisterTypeMigration() was called
+// prior to RegisterWrapperEncoder().
+func RegisterWrapperEncoderWithMessageType(
+	theType TypeKey, encoder WrapperEncoderWithMessageType,
+) {
 	if encoder == nil {
 		delete(encoders, theType)
 	} else {
@@ -304,7 +357,42 @@ 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,
+)
+
+// MessageType is used to encode information about an error message
+// within a wrapper error type. This information is used to affect
+// display logic.
+type MessageType errorspb.MessageType
+
+// Values below should match the ones in errorspb.MessageType for
+// direct conversion.
+const (
+	// Prefix denotes an error message that should be prepended to the
+	// message of its cause.
+	Prefix MessageType = MessageType(errorspb.MessageType_PREFIX)
+	// FullMessage denotes an error message that contains the text of its
+	// causes and can be displayed standalone.
+	FullMessage = MessageType(errorspb.MessageType_FULL_MESSAGE)
+)
+
+// WrapperEncoderWithMessageType is to be provided (via
+// RegisterWrapperEncoderWithMessageType above) by additional wrapper
+// types not yet known to this library. This encoder returns an
+// additional enum which indicates whether the wrapper owns the error
+// message completely instead of simply being a prefix with the error
+// message of its causes appended to it. This information is encoded
+// along with the prefix in order to provide context during error
+// display.
+type WrapperEncoderWithMessageType func(ctx context.Context, err error) (
+	msgPrefix string,
+	safeDetails []string,
+	payload proto.Message,
+	messageType MessageType,
+)
 
 // registry for RegisterWrapperType.
-var encoders = map[TypeKey]WrapperEncoder{}
+var encoders = map[TypeKey]WrapperEncoderWithMessageType{}

errbase/err_string_ownership_test.go

@@ -0,0 +1,105 @@
+// Copyright 2023 The Cockroach Authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+// implied. See the License for the specific language governing
+// permissions and limitations under the License.
+
+package errbase_test
+
+import (
+	"context"
+	goErr "errors"
+	"fmt"
+	"testing"
+
+	"github.com/cockroachdb/errors/errbase"
+	"github.com/cockroachdb/errors/errorspb"
+	"github.com/cockroachdb/errors/testutils"
+)
+
+func genEncoded(mt errorspb.MessageType) errorspb.EncodedError {
+	return errorspb.EncodedError{
+		Error: &errorspb.EncodedError_Wrapper{
+			Wrapper: &errorspb.EncodedWrapper{
+				Cause: errorspb.EncodedError{
+					Error: &errorspb.EncodedError_Leaf{
+						Leaf: &errorspb.EncodedErrorLeaf{
+							Message: "leaf-error-msg",
+						},
+					},
+				},
+				Message:     "wrapper-error-msg: leaf-error-msg: extra info",
+				Details:     errorspb.EncodedErrorDetails{},
+				MessageType: mt,
+			},
+		},
+	}
+}
+
+func TestDecodeOldVersion(t *testing.T) {
+	tt := testutils.T{T: t}
+
+	errOldEncoded := genEncoded(errorspb.MessageType_PREFIX)
+	errOldDecoded := errbase.DecodeError(context.Background(), errOldEncoded)
+	// Ensure that we will continue to just concat leaf after wrapper
+	// with older errors for backward compatibility.
+	tt.CheckEqual(errOldDecoded.Error(), "wrapper-error-msg: leaf-error-msg: extra info: leaf-error-msg")
+
+	// Check to ensure that when flag is present, we interpret things correctly.
+	errNewEncoded := genEncoded(errorspb.MessageType_FULL_MESSAGE)
+	errNewDecoded := errbase.DecodeError(context.Background(), errNewEncoded)
+	tt.CheckEqual(errNewDecoded.Error(), "wrapper-error-msg: leaf-error-msg: extra info")
+}
+
+func TestEncodeDecodeNewVersion(t *testing.T) {
+	tt := testutils.T{T: t}
+	errNewEncoded := errbase.EncodeError(
+		context.Background(),
+		fmt.Errorf(
+			"wrapper-error-msg: %w: extra info",
+			goErr.New("leaf-error-msg"),
+		),
+	)
+
+	errNew := errorspb.EncodedError{
+		Error: &errorspb.EncodedError_Wrapper{
+			Wrapper: &errorspb.EncodedWrapper{
+				Cause: errorspb.EncodedError{
+					Error: &errorspb.EncodedError_Leaf{
+						Leaf: &errorspb.EncodedErrorLeaf{
+							Message: "leaf-error-msg",
+							Details: errorspb.EncodedErrorDetails{
+								OriginalTypeName:  "errors/*errors.errorString",
+								ErrorTypeMark:     errorspb.ErrorTypeMark{FamilyName: "errors/*errors.errorString", Extension: ""},
+								ReportablePayload: nil,
+								FullDetails:       nil,
+							},
+						},
+					},
+				},
+				Message: "wrapper-error-msg: leaf-error-msg: extra info",
+				Details: errorspb.EncodedErrorDetails{
+					OriginalTypeName:  "fmt/*fmt.wrapError",
+					ErrorTypeMark:     errorspb.ErrorTypeMark{FamilyName: "fmt/*fmt.wrapError", Extension: ""},
+					ReportablePayload: nil,
+					FullDetails:       nil,
+				},
+				MessageType: errorspb.MessageType_FULL_MESSAGE,
+			},
+		},
+	}
+
+	tt.CheckDeepEqual(errNewEncoded, errNew)
+	newErr := errbase.DecodeError(context.Background(), errNew)
+
+	// New version correctly decodes error
+	tt.CheckEqual(newErr.Error(), "wrapper-error-msg: leaf-error-msg: extra info")
+}