JSON unmarshaling by default errors on unknown field inputs.

[koblas]

Is your feature request related to a problem? Please describe. I'm porting a bunch of infrastructure from twirp to connect and one of the things I just got caught by was the JSON Codec for Service Unmarshaling by default signals an error for fields present in JSON not present in the protobuf description.

Describe the solution you'd like Either:

1. By default use an options with discard set true (e.g.options := protojson.UnmarshalOptions{DiscardUnknown: true}) by default.
2. Have a way to configure options into the standard Codecs -- maybe as simple as connect.WithJsonDiscardUnknown().

Describe alternatives you've considered My current solution was to cut-n-paste out the JSON codec and provide it back in as an option to my service handler.

    _, api := apiv1connect.NewUserServiceHandler(
        user.NewUserServer(config, opts...),
        connect.WithCodec(bufcutil.NewJsonCodec()),
    )

Additional context

Nope.

[akshayjshah]

This is pretty reasonable - we ought to make this sort of customization more straightforward.

We're definitely not going to make discarding unknown fields the default behavior. It doesn't seem clearly better than the current behavior, it's a significant change post-1.0, and it doesn't match the default for protojson.

Tentatively, I think it's best not to special-case this one toggle. If we're going to do anything, I'd prefer WithJSONMarshalOptions and WithJSONUnmarshalOptions. If you'd like to open a PR to add those two options, I'm happy to take a look. If not, we'll get back to this but it may take a little while - we're focused on adding support for HTTP GETs right now.

[akshayjshah]

After thinking about this a bit more, I'd prefer to leave this configurability to third-party packages. From the discussion on #500:

In general, I'm reluctant to expose APIs from connect-go that don't require access to significant chunks of the package internals. Over time, this keeps the surface area of connect-go as small and approachable as possible.

The protojson codec is so small that I don't think this API carries its weight. It's relatively easy, and just as functional, to provide this configurability from a separate package that replicates the JSON marshaling logic. I've done just that, so you can use go.akshayshah.org/connectproto today. (Please do keep in mind that this is a small personal repo rather than an official Buf project!)

We're sorting out the best way to make third-party packages like these more discoverable, but hopefully this unblocks you in the short term.

In short, use go.akshayshah.org/connectproto until we settle on a more official solution.

[koblas]

I'm hopeful that this gets incorporated at least into and official buf package in the future.

thanks.

[pkwarren]

I think it would be good to configure connect-go so that servers reject unknown JSON fields by default, however clients should ignore unknown fields. That allows a server to implement a new API or add new fields over time without having every client update to the latest version immediately (creating a tight coupling between the two).

[koblas]

@pkwarren that's a great point. Technically the protobuf spec allows fields to be added and removed as you update your implementation. The current implementaiton for JSON doesn't follow that phlosophy, but rather pedantically requires you to have all clients match your current server implementation.

[timostamm]

@koblas, the protobuf language guide says that a "Proto3 JSON parser should reject unknown fields by default", so it is not perfectly straight-forward to find the least surprising behavior.

That said, @pkwarren's suggestion seems practical. https://github.com/bufbuild/connect-es/issues/613 is a case where a user was surprised by the same default behavior in connect-es clients.

[akshayjshah]

Y'all have convinced me - I hadn't considered how tightly the current behavior couples servers and clients. We should definitely make clients discard unknown fields. Once we've done that, it seems pretty reasonable to have servers do the same - they already do so for binary inputs, and it seems reasonable to accommodate clients sending old fields that are now deleted (and reserved).