BadRequest Field FieldViolation is in a debug member (unexpected location)

[koblas]

Describe the bug

I was setting up error handling for fields (missing email address in this example). When the error was serialized via JSON the error was not in the details array directly but in a debug member of the entry. I was expecting this to be serialized in more top level entry. While I don't have a JSON representation around from when I was working with GRPC-JSON from golang, the code I didn't use the debug member.

Note: this comes from protocol_conect.go func (d *connectWireDetail) MarshalJSON() ([]byte, error) {

To Reproduce

When the following code is used to build a BadRequest + FieldViolation error message, the result is given in JSON.

func InvalidArgumentError(field, msg string) *connect.Error {
    e := connect.NewError(connect.CodeInvalidArgument, errors.New(msg))

    data := &errdetails.BadRequest{
        FieldViolations: []*errdetails.BadRequest_FieldViolation{
            {
                Field:       field,
                Description: msg,
            },
        },
    }
    if detail, err := connect.NewErrorDetail(data); err == nil {
        e.AddDetail(detail)
    }

    return e
}

{
   "code" : "invalid_argument",
   "details" : [
      {
         "debug" : {
            "@type" : "type.googleapis.com/google.rpc.BadRequest",
            "fieldViolations" : [
               {
                  "description" : "Bad email or password (empty_field)",
                  "field" : "email"
               }
            ]
         },
         "type" : "google.rpc.BadRequest",
         "value" : "CiwKBWVtYWlsEiNCYWQgZW1haWwgb3IgcGFzc3dvcmQgKGVtcHR5X2ZpZWxkKQ"
      }
   ],
   "message" : "Bad email or password (empty_field)"
}

Environment (please complete the following information):

• connect-go v1.5.2
• go v1.19
• your complete go.mod:

module github.com/koblas/grpc-todo

go 1.19

require (
    github.com/adjust/rmq/v5 v5.0.1
    github.com/aead/chacha20poly1305 v0.0.0-20201124145622-1a5aba2a8b29
    github.com/aws/aws-cdk-go/awscdk/v2 v2.45.0
    github.com/aws/aws-lambda-go v1.28.0
    github.com/aws/aws-sdk-go v1.42.22
    github.com/aws/aws-sdk-go-v2 v1.17.3
    github.com/aws/aws-sdk-go-v2/config v1.11.0
    github.com/aws/aws-sdk-go-v2/credentials v1.6.4
    github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue v1.4.4
    github.com/aws/aws-sdk-go-v2/service/apigatewaymanagementapi v1.8.0
    github.com/aws/aws-sdk-go-v2/service/dynamodb v1.10.0
    github.com/aws/aws-sdk-go-v2/service/lambda v1.14.1
    github.com/aws/aws-sdk-go-v2/service/s3 v1.29.6
    github.com/aws/aws-sdk-go-v2/service/sns v1.12.1
    github.com/aws/aws-sdk-go-v2/service/sqs v1.14.0
    github.com/aws/aws-sdk-go-v2/service/ssm v1.17.1
    github.com/aws/constructs-go/constructs/v10 v10.1.124
    github.com/aws/jsii-runtime-go v1.69.0
    github.com/aws/smithy-go v1.13.5
    github.com/dgrijalva/jwt-go v3.2.0+incompatible
    github.com/disintegration/imaging v1.6.2
    github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1
    github.com/fnproject/fdk-go v0.0.21
    github.com/go-playground/validator/v10 v10.11.1
    github.com/go-redis/redis/v8 v8.11.4
    github.com/google/uuid v1.3.0
    github.com/gorilla/websocket v1.5.0
    github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
    github.com/jaswdr/faker v1.8.0
    github.com/json-iterator/go v1.1.12
    github.com/minio/minio-go/v7 v7.0.47
    github.com/nats-io/nats.go v1.22.1
    github.com/o1egl/paseto v1.0.0
    github.com/oklog/ulid/v2 v2.1.0
    github.com/pkg/errors v0.9.1
    github.com/renstrom/shortuuid v3.0.0+incompatible
    github.com/robinjoseph08/redisqueue v1.1.0
    github.com/rs/xid v1.4.0
    github.com/stretchr/testify v1.8.2
    github.com/tencentyun/scf-go-lib v0.0.0-20211123032342-f972dcd16ff6
    github.com/twitchtv/twirp v8.1.1+incompatible
    go.uber.org/zap v1.19.1
    golang.org/x/crypto v0.5.0
    golang.org/x/net v0.5.0
    golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8
    golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4
    google.golang.org/genproto v0.0.0-20211102202547-e9cf271f7f2c
    google.golang.org/grpc v1.51.0
    google.golang.org/protobuf v1.28.1
    gopkg.in/gomail.v2 v2.0.0-20160411212932-81ebce5c23df
)

require (
    cloud.google.com/go v0.65.0 // indirect
    github.com/Masterminds/semver/v3 v3.1.1 // indirect
    github.com/PullRequestInc/go-gpt3 v1.1.11 // indirect
    github.com/aead/chacha20 v0.0.0-20180709150244-8b13a72661da // indirect
    github.com/aead/poly1305 v0.0.0-20180717145839-3fee0db0b635 // indirect
    github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.4.10 // indirect
    github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.8.2 // indirect
    github.com/aws/aws-sdk-go-v2/internal/configsources v1.1.27 // indirect
    github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.4.21 // indirect
    github.com/aws/aws-sdk-go-v2/internal/ini v1.3.2 // indirect
    github.com/aws/aws-sdk-go-v2/internal/v4a v1.0.18 // indirect
    github.com/aws/aws-sdk-go-v2/service/dynamodbstreams v1.8.1 // indirect
    github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.9.11 // indirect
    github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.1.22 // indirect
    github.com/aws/aws-sdk-go-v2/service/internal/endpoint-discovery v1.3.3 // indirect
    github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.9.21 // indirect
    github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.13.21 // indirect
    github.com/aws/aws-sdk-go-v2/service/sso v1.6.2 // indirect
    github.com/aws/aws-sdk-go-v2/service/sts v1.11.1 // indirect
    github.com/bufbuild/connect-go v1.5.2 // indirect
    github.com/bufbuild/connect-grpchealth-go v1.0.0 // indirect
    github.com/cespare/xxhash/v2 v2.1.2 // indirect
    github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1 // indirect
    github.com/davecgh/go-spew v1.1.1 // indirect
    github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
    github.com/dustin/go-humanize v1.0.0 // indirect
    github.com/envoyproxy/protoc-gen-validate v0.1.0 // indirect
    github.com/fatih/color v1.7.0 // indirect
    github.com/git-chglog/git-chglog v0.0.0-20190611050339-63a4e637021f // indirect
    github.com/go-logr/logr v1.2.3 // indirect
    github.com/go-logr/stdr v1.2.2 // indirect
    github.com/go-playground/locales v0.14.0 // indirect
    github.com/go-playground/universal-translator v0.18.0 // indirect
    github.com/go-redis/redis v6.15.2+incompatible // indirect
    github.com/golang/protobuf v1.5.2 // indirect
    github.com/imdario/mergo v0.3.7 // indirect
    github.com/jmespath/go-jmespath v0.4.0 // indirect
    github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 // indirect
    github.com/klauspost/compress v1.15.11 // indirect
    github.com/klauspost/cpuid/v2 v2.1.0 // indirect
    github.com/leodido/go-urn v1.2.1 // indirect
    github.com/mattn/go-colorable v0.1.2 // indirect
    github.com/mattn/go-isatty v0.0.16 // indirect
    github.com/mattn/goveralls v0.0.2 // indirect
    github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b // indirect
    github.com/minio/md5-simd v1.1.2 // indirect
    github.com/minio/sha256-simd v1.0.0 // indirect
    github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect
    github.com/modern-go/reflect2 v1.0.2 // indirect
    github.com/nats-io/nats-server/v2 v2.9.11 // indirect
    github.com/nats-io/nkeys v0.3.0 // indirect
    github.com/nats-io/nuid v1.0.1 // indirect
    github.com/opentracing/opentracing-go v1.2.0 // indirect
    github.com/pmezard/go-difflib v1.0.0 // indirect
    github.com/sirupsen/logrus v1.9.0 // indirect
    github.com/tsuyoshiwada/go-gitcmd v0.0.0-20180205145712-5f1f5f9475df // indirect
    github.com/urfave/cli v1.20.0 // indirect
    github.com/yuin/goldmark v1.4.13 // indirect
    go.opentelemetry.io/otel v1.14.0 // indirect
    go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.14.0 // indirect
    go.opentelemetry.io/otel/sdk v1.14.0 // indirect
    go.opentelemetry.io/otel/trace v1.14.0 // indirect
    go.uber.org/atomic v1.7.0 // indirect
    go.uber.org/multierr v1.6.0 // indirect
    golang.org/x/image v0.3.0 // indirect
    golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 // indirect
    golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
    golang.org/x/sys v0.5.0 // indirect
    golang.org/x/text v0.6.0 // indirect
    golang.org/x/tools v0.1.12 // indirect
    google.golang.org/appengine v1.6.6 // indirect
    gopkg.in/AlecAivazis/survey.v1 v1.8.5 // indirect
    gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect
    gopkg.in/ini.v1 v1.66.6 // indirect
    gopkg.in/kyokomi/emoji.v1 v1.5.1 // indirect
    gopkg.in/yaml.v2 v2.4.0 // indirect
    gopkg.in/yaml.v3 v3.0.1 // indirect
)

Additional context

n/a

[timostamm]

Hey David, it would be convenient to have just the information under the "debug" field, I agree.

We cannot do that because we want to enable protocol transcoding between Connect and gRCP or gRCP-web. The latter two store error details as a google.rpc.Status message in a grpc-status-details-bin trailer in binary format. To allow middleboxes without knowledge about the schema to transcode to or from Connect, we have to use binary format as well.

We do provide the JSON representation if we can, but it is optional. Quoting from the protocol spec:

Each detail is an object with "type" and "value" properties and any number of other properties. The "type" field contains the fully-qualified Protobuf message name as a UTF-8 string, and the "value" field contains unpadded, base64-encoded binary Protobuf data. For readability on the wire, server implementations may also serialize the detail to JSON and include the resulting object under the "debug" key. Clients must not depend on data in the "debug" key when deserializing details.

I'm closing this issue because protocol transcoding is an important feature we want to retain.

[akshayjshah]

Everything Timo said is 100% correct. David, some more context on the gRPC protocol may be helpful...

Error details are not formally part of the gRPC protocol specification, despite being important and widely used. Because they were added before the gRFC process existed, there's very little documentation - you have to read through the various implementations to sort out how they work. They're also a little odd - it's the only place where gRPC is irrevocably tied to Protocol Buffers. No matter what codec you use, error details are always sent over the wire as base64-encoded binary protobuf. 🤷🏽‍♂️

To let proxies, net/http middleware, and other systems without access to schemas translate the Connect protocol to and from gRPC, we need to send binary on the wire too. This is one of the two main concessions we had to make to maintain parity between the two protocols (the other is reusing gRPC's status codes).