diff --git a/markers/markers.go b/markers/markers.go index 50a16e8..4500338 100644 --- a/markers/markers.go +++ b/markers/markers.go @@ -268,11 +268,13 @@ func encodeMark(_ context.Context, err error) (msg string, _ []string, payload p func decodeMark(_ context.Context, cause error, _ string, _ []string, payload proto.Message) error { m, ok := payload.(*errorspb.MarkPayload) - if !ok { + if !ok || len(m.Types) == 0 { // If this ever happens, this means some version of the library - // (presumably future) changed the payload type, and we're - // receiving this here. In this case, give up and let - // DecodeError use the opaque type. + // (presumably future) changed the payload type or stopped + // filling in the type marks, and we're receiving this here. In + // this case, give up and let DecodeError use the opaque type. + // A mark without any type would also break SafeFormatError + // below, which reports types[0]. return nil } return &withMark{cause: cause, mark: errorMark{msg: m.Msg, types: m.Types}} diff --git a/markers/markers_test.go b/markers/markers_test.go index 657c8d8..d1b8913 100644 --- a/markers/markers_test.go +++ b/markers/markers_test.go @@ -24,8 +24,10 @@ import ( "testing" "github.com/cockroachdb/errors/errbase" + "github.com/cockroachdb/errors/errorspb" "github.com/cockroachdb/errors/markers" "github.com/cockroachdb/errors/testutils" + "github.com/gogo/protobuf/types" pkgErr "github.com/pkg/errors" ) @@ -803,3 +805,31 @@ func (errorUncomparable) Is(target error) bool { _, ok := target.(errorUncomparable) return ok } + +// TestDecodeMarkWithoutTypes checks that a MarkPayload that arrives +// without any type marks does not break the formatting of the decoded +// error. The payload is built by hand because the local encoder always +// emits at least one type mark; a peer running a different version of +// the library is not bound by that. +func TestDecodeMarkWithoutTypes(t *testing.T) { + ctx := context.Background() + + enc := errbase.EncodeError(ctx, markers.Mark(errors.New("boom"), errors.New("refmsg"))) + w := enc.GetWrapper() + if w == nil { + t.Fatalf("expected a wrapper, got %+v", enc) + } + payload, err := types.MarshalAny(&errorspb.MarkPayload{Msg: "refmsg"}) + if err != nil { + t.Fatal(err) + } + w.Details.FullDetails = payload + + decoded := errbase.DecodeError(ctx, enc) + if s := decoded.Error(); s != "boom" { + t.Errorf("Error(): expected %q, got %q", "boom", s) + } + if s := fmt.Sprintf("%+v", decoded); strings.Contains(s, "PANIC=") { + t.Errorf("formatting the decoded error panicked: %s", s) + } +}