Drop @type from Connect error detail debug string (#688)

This makes the output of a connect-go server look more like the examples
in the docs. Previously, the debug propery of JSON error details included
a superfluous "@type" attribute that was not described in the docs.

error.go

@@ -49,8 +49,9 @@ var (
 // The [google.golang.org/genproto/googleapis/rpc/errdetails] package contains a
 // variety of Protobuf messages commonly used as error details.
 type ErrorDetail struct {
-	pb       *anypb.Any
-	wireJSON string // preserve human-readable JSON
+	pbAny    *anypb.Any
+	pbInner  proto.Message // if nil, must be extracted from pbAny
+	wireJSON string        // preserve human-readable JSON
 }
 
 // NewErrorDetail constructs a new error detail. If msg is an *[anypb.Any] then
@@ -59,13 +60,13 @@ type ErrorDetail struct {
 func NewErrorDetail(msg proto.Message) (*ErrorDetail, error) {
 	// If it's already an Any, don't wrap it inside another.
 	if pb, ok := msg.(*anypb.Any); ok {
-		return &ErrorDetail{pb: pb}, nil
+		return &ErrorDetail{pbAny: pb}, nil
 	}
 	pb, err := anypb.New(msg)
 	if err != nil {
 		return nil, err
 	}
-	return &ErrorDetail{pb: pb}, nil
+	return &ErrorDetail{pbAny: pb, pbInner: msg}, nil
 }
 
 // Type is the fully-qualified name of the detail's Protobuf message (for
@@ -79,21 +80,26 @@ func (d *ErrorDetail) Type() string {
 	//
 	// If we ever want to support remote registries, we can add an explicit
 	// `TypeURL` method.
-	return typeNameFromURL(d.pb.GetTypeUrl())
+	return typeNameFromURL(d.pbAny.GetTypeUrl())
 }
 
 // Bytes returns a copy of the Protobuf-serialized detail.
 func (d *ErrorDetail) Bytes() []byte {
-	out := make([]byte, len(d.pb.GetValue()))
-	copy(out, d.pb.GetValue())
+	out := make([]byte, len(d.pbAny.GetValue()))
+	copy(out, d.pbAny.GetValue())
 	return out
 }
 
 // Value uses the Protobuf runtime's package-global registry to unmarshal the
 // Detail into a strongly-typed message. Typically, clients use Go type
 // assertions to cast from the proto.Message interface to concrete types.
 func (d *ErrorDetail) Value() (proto.Message, error) {
-	return d.pb.UnmarshalNew()
+	if d.pbInner != nil {
+		// We clone it so that if the caller mutates the returned value,
+		// they don't inadvertently corrupt this error detail value.
+		return proto.Clone(d.pbInner), nil
+	}
+	return d.pbAny.UnmarshalNew()
 }
 
 // An Error captures four key pieces of information: a [Code], an underlying Go
@@ -236,7 +242,7 @@ func (e *Error) Meta() http.Header {
 func (e *Error) detailsAsAny() []*anypb.Any {
 	anys := make([]*anypb.Any, 0, len(e.details))
 	for _, detail := range e.details {
-		anys = append(anys, detail.pb)
+		anys = append(anys, detail.pbAny)
 	}
 	return anys
 }

protocol_connect.go

@@ -30,6 +30,7 @@ import (
 	"strings"
 	"time"
 
+	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/known/anypb"
 )
 
@@ -1146,15 +1147,18 @@ func (d *connectWireDetail) MarshalJSON() ([]byte, error) {
 		Value string          `json:"value"`
 		Debug json.RawMessage `json:"debug,omitempty"`
 	}{
-		Type:  typeNameFromURL(d.pb.GetTypeUrl()),
-		Value: base64.RawStdEncoding.EncodeToString(d.pb.GetValue()),
+		Type:  typeNameFromURL(d.pbAny.GetTypeUrl()),
+		Value: base64.RawStdEncoding.EncodeToString(d.pbAny.GetValue()),
 	}
 	// Try to produce debug info, but expect failure when we don't have
 	// descriptors.
-	var codec protoJSONCodec
-	debug, err := codec.Marshal(d.pb)
-	if err == nil && len(debug) > 2 { // don't bother sending `{}`
-		wire.Debug = json.RawMessage(debug)
+	msg, err := d.getInner()
+	if err == nil {
+		var codec protoJSONCodec
+		debug, err := codec.Marshal(msg)
+		if err == nil {
+			wire.Debug = debug
+		}
 	}
 	return json.Marshal(wire)
 }
@@ -1175,7 +1179,7 @@ func (d *connectWireDetail) UnmarshalJSON(data []byte) error {
 		return fmt.Errorf("decode base64: %w", err)
 	}
 	*d = connectWireDetail{
-		pb: &anypb.Any{
+		pbAny: &anypb.Any{
 			TypeUrl: wire.Type,
 			Value:   decoded,
 		},
@@ -1184,6 +1188,13 @@ func (d *connectWireDetail) UnmarshalJSON(data []byte) error {
 	return nil
 }
 
+func (d *connectWireDetail) getInner() (proto.Message, error) {
+	if d.pbInner != nil {
+		return d.pbInner, nil
+	}
+	return d.pbAny.UnmarshalNew()
+}
+
 type connectWireError struct {
 	Code    Code                 `json:"code"`
 	Message string               `json:"message,omitempty"`

protocol_connect_test.go

@@ -24,30 +24,67 @@ import (
 	"time"
 
 	"connectrpc.com/connect/internal/assert"
+	"google.golang.org/protobuf/proto"
+	"google.golang.org/protobuf/types/descriptorpb"
 	"google.golang.org/protobuf/types/known/durationpb"
 )
 
 func TestConnectErrorDetailMarshaling(t *testing.T) {
 	t.Parallel()
-	detail, err := NewErrorDetail(durationpb.New(time.Second))
-	assert.Nil(t, err)
-	data, err := json.Marshal((*connectWireDetail)(detail))
-	assert.Nil(t, err)
-	t.Logf("marshaled error detail: %s", string(data))
+	testCases := []struct {
+		name        string
+		errorDetail proto.Message
+		expectDebug any
+	}{
+		{
+			name: "normal",
+			errorDetail: &descriptorpb.FieldOptions{
+				Deprecated: proto.Bool(true),
+				Jstype:     descriptorpb.FieldOptions_JS_STRING.Enum(),
+			},
+			expectDebug: map[string]any{
+				"deprecated": true,
+				"jstype":     "JS_STRING",
+			},
+		},
+		{
+			name:        "well-known type with custom JSON",
+			errorDetail: durationpb.New(time.Second),
+			expectDebug: "1s", // special JS representation as duration string
+		},
+	}
+	for _, testCase := range testCases {
+		testCase := testCase
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Parallel()
+
+			detail, err := NewErrorDetail(testCase.errorDetail)
+			assert.Nil(t, err)
+			data, err := json.Marshal((*connectWireDetail)(detail))
+			assert.Nil(t, err)
+			t.Logf("marshaled error detail: %s", string(data))
 
-	var unmarshaled connectWireDetail
-	assert.Nil(t, json.Unmarshal(data, &unmarshaled))
-	assert.Equal(t, unmarshaled.wireJSON, string(data))
-	assert.Equal(t, unmarshaled.pb, detail.pb)
+			var unmarshaled connectWireDetail
+			assert.Nil(t, json.Unmarshal(data, &unmarshaled))
+			assert.Equal(t, unmarshaled.wireJSON, string(data))
+			assert.Equal(t, unmarshaled.pbAny, detail.pbAny)
+
+			var extractDetails struct {
+				Debug any `json:"debug"`
+			}
+			assert.Nil(t, json.Unmarshal(data, &extractDetails))
+			assert.Equal(t, extractDetails.Debug, testCase.expectDebug)
+		})
+	}
 }
 
 func TestConnectErrorDetailMarshalingNoDescriptor(t *testing.T) {
 	t.Parallel()
 	raw := `{"type":"acme.user.v1.User","value":"DEADBF",` +
-		`"debug":{"@type":"acme.user.v1.User","email":"someone@connectrpc.com"}}`
+		`"debug":{"email":"someone@connectrpc.com"}}`
 	var detail connectWireDetail
 	assert.Nil(t, json.Unmarshal([]byte(raw), &detail))
-	assert.Equal(t, detail.pb.GetTypeUrl(), defaultAnyResolverPrefix+"acme.user.v1.User")
+	assert.Equal(t, detail.pbAny.GetTypeUrl(), defaultAnyResolverPrefix+"acme.user.v1.User")
 
 	_, err := (*ErrorDetail)(&detail).Value()
 	assert.NotNil(t, err)

protocol_grpc.go

@@ -793,7 +793,7 @@ func grpcErrorFromTrailer(protobuf Codec, trailer http.Header) *Error {
 			return errorf(CodeInternal, "server returned invalid protobuf for error details: %w", err)
 		}
 		for _, d := range status.GetDetails() {
-			retErr.details = append(retErr.details, &ErrorDetail{pb: d})
+			retErr.details = append(retErr.details, &ErrorDetail{pbAny: d})
 		}
 		// Prefer the Protobuf-encoded data to the headers (grpc-go does this too).
 		retErr.code = Code(status.GetCode())