diff --git a/connect_ext_test.go b/connect_ext_test.go index 86bdeeff..169b417f 100644 --- a/connect_ext_test.go +++ b/connect_ext_test.go @@ -690,7 +690,11 @@ func TestGRPCMarshalStatusError(t *testing.T) { mux := http.NewServeMux() mux.Handle(pingv1connect.NewPingServiceHandler( - pingServer{}, + pingServer{ + // Include error details in the response, so that the Status protobuf will be marshaled. + includeErrorDetails: true, + }, + // We're using a codec that will fail to marshal the Status protobuf, which means the returned error will be ignored connect.WithCodec(failCodec{}), )) server := memhttptest.NewServer(t, mux) @@ -701,11 +705,12 @@ func TestGRPCMarshalStatusError(t *testing.T) { request := connect.NewRequest(&pingv1.FailRequest{Code: int32(connect.CodeResourceExhausted)}) _, err := client.Fail(context.Background(), request) tb.Log(err) - assert.NotNil(t, err) + assert.NotNil(t, err, assert.Sprintf("expected an error")) var connectErr *connect.Error ok := errors.As(err, &connectErr) - assert.True(t, ok) - assert.Equal(t, connectErr.Code(), connect.CodeInternal) + assert.True(t, ok, assert.Sprintf("expected the error to be a connect.Error")) + // This should be Internal, not ResourceExhausted, because we're testing when the Status object itself fails to marshal + assert.Equal(t, connectErr.Code(), connect.CodeInternal, assert.Sprintf("expected the error code to be Internal, was %s", connectErr.Code())) assert.True( t, strings.HasSuffix(connectErr.Message(), ": boom"), @@ -2609,7 +2614,8 @@ func expectMetadata(meta http.Header, metaType, key, value string) error { type pingServer struct { pingv1connect.UnimplementedPingServiceHandler - checkMetadata bool + checkMetadata bool + includeErrorDetails bool } func (p pingServer) Ping(ctx context.Context, request *connect.Request[pingv1.PingRequest]) (*connect.Response[pingv1.PingResponse], error) { @@ -2646,6 +2652,13 @@ func (p pingServer) Fail(ctx context.Context, request *connect.Request[pingv1.Fa err := connect.NewError(connect.Code(request.Msg.GetCode()), errors.New(errorMessage)) err.Meta().Set(handlerHeader, headerValue) err.Meta().Set(handlerTrailer, trailerValue) + if p.includeErrorDetails { + detail, derr := connect.NewErrorDetail(&pingv1.FailRequest{Code: request.Msg.GetCode()}) + if derr != nil { + return nil, derr + } + err.AddDetail(detail) + } return nil, err } diff --git a/protocol_grpc.go b/protocol_grpc.go index 5a58cf42..d498db59 100644 --- a/protocol_grpc.go +++ b/protocol_grpc.go @@ -838,33 +838,30 @@ func grpcContentTypeFromCodecName(web bool, name string) string { func grpcErrorToTrailer(trailer http.Header, protobuf Codec, err error) { if err == nil { setHeaderCanonical(trailer, grpcHeaderStatus, "0") // zero is the gRPC OK status - setHeaderCanonical(trailer, grpcHeaderMessage, "") - return - } - status := grpcStatusFromError(err) - code := strconv.Itoa(int(status.GetCode())) - bin, binErr := protobuf.Marshal(status) - if binErr != nil { - setHeaderCanonical( - trailer, - grpcHeaderStatus, - strconv.FormatInt(int64(CodeInternal), 10 /* base */), - ) - setHeaderCanonical( - trailer, - grpcHeaderMessage, - grpcPercentEncode( - fmt.Sprintf("marshal protobuf status: %v", binErr), - ), - ) return } if connectErr, ok := asError(err); ok { mergeHeaders(trailer, connectErr.meta) } - setHeaderCanonical(trailer, grpcHeaderStatus, code) - setHeaderCanonical(trailer, grpcHeaderMessage, grpcPercentEncode(status.GetMessage())) - setHeaderCanonical(trailer, grpcHeaderDetails, EncodeBinaryHeader(bin)) + var ( + status = grpcStatusFromError(err) + code = status.GetCode() + message = status.GetMessage() + bin []byte + ) + if len(status.Details) > 0 { + var binErr error + bin, binErr = protobuf.Marshal(status) + if binErr != nil { + code = int32(CodeInternal) + message = fmt.Sprintf("marshal protobuf status: %v", binErr) + } + } + setHeaderCanonical(trailer, grpcHeaderStatus, strconv.Itoa(int(code))) + setHeaderCanonical(trailer, grpcHeaderMessage, grpcPercentEncode(message)) + if len(bin) > 0 { + setHeaderCanonical(trailer, grpcHeaderDetails, EncodeBinaryHeader(bin)) + } } func grpcStatusFromError(err error) *statusv1.Status {