Unable to specify empty response schema

[aleert]

🐛 Bug Report

Response schema states that

// `Schema` optionally defines the structure of the response.
  // If `Schema` is not provided, it means there is no content to the response.
  Schema schema = 2;

however when i omit schema from rpc response defenition it still renders as "schema": {} in api.swagger.json which in editor rendered as

To Reproduce

Define RPC with empty schema response option

 option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = {
      responses: {
        key: "202"
        value: {
          description: "test description",
        },
      };
  }

Expected behavior

schema key not renders in api.swagger.json and rendered schema looks like

[johanbrandhorst]

Have you tried using google.protobuf.Empty as the response type?

[aleert]

Have you tried using google.protobuf.Empty as the response type?

Yes, it displays empty message as example response value and has description according to google.protobuf.Empty docs as schema key in api.swagger.json is still generated.

[johanbrandhorst]

I had a look at the spec, and I agree that it should be possible to set this to something empty given this definition of the field:

However, with the way that we specify responses using the annotations, how would we tell the difference between an empty response and a response that's not been set at all? The message definition is here: https://buf.build/grpc-ecosystem/grpc-gateway/file/main:protoc-gen-openapiv2/options/openapiv2.proto#L230. Setting this to null is the same as not setting it at all, and we currently only let this overwrite the default response value (inferred from the type of the response) if it's set, so I don't see how we could possible support this given our existing annotations. Have you got any ideas?

[same-id]

Hey,
Pitching in.

BTW, buf links are not permanent (https://buf.build/grpc-ecosystem/grpc-gateway/file/main:protoc-gen-openapiv2/options/openapiv2.proto#L230)

But if we meant this:

grpc-gateway/protoc-gen-openapiv2/options/openapiv2.proto

Line 280 in 67896a8

Schema schema = 2;

Can't we change Schema schema = 2 to optional Schema schema = 2?

However I understand that (https://google.aip.dev/180#moving-into-oneofs):

Existing fields must not be moved into or out of a oneof. This is a backwards-incompatible change in the Go protobuf stubs.

And optional=oneof

[johanbrandhorst]

I'd be worried about making a change like that to a type that's so central to all our annotations, and I'm not convinced it would make a difference either. If we can already set it to null today, what is optional going to add? But feel free to experiment with it 😄.

[same-id]

In this case, this is a breaking change, but however I think it's fine since I guess no one is using those annotations outside of grpc-gateway Go generated files - so I guess that the blast radius is confined.

To be honest, this example is a microcosmos of actual grpc trancoding challanges (since we want our annotation protobuf to have the same structure as the schema json): Since in json files you can specify the absence of the field as another state, but protobuf tooling auto generates empty classes/structs in all languages even if they are missing from the wire.

Back to the task at hand,
How do I make sure that there is no response generated at all, at least for documentation purposes (even if API does return {})

[johanbrandhorst]

Would disable_default_responses be useful? https://github.com/grpc-ecosystem/grpc-gateway/blob/main/protoc-gen-openapiv2/main.go#L44

[same-id]

Actually this is going too strong in the other direction.
Default 200 responses are not auto generated for the 99 out of the 100 API's which are not google.protobuf.Empty
(One would say that I got 99 problems and google.protobuf.Empty ain't one)

I think there are two ways to solve the issue in a more reasonable way:

1. Add optional on the Schema field. This is a breaking change but probably no one uses generated annotation code other than grpc-gateway itself. If we don't like it, we can add "yet-another-feature-flag" or add a manual "has_schema" to the protobuf definition.
2. We can treat google.protobuf.Empty specially and not specify a returned schema. However if we just return a non myorg.protobuf.Empty it will still show {} as the response.

To be honest I'm also not so sure about if it's okay to document that an API call does not return content, when in practice it does, this is a limitation of gRPC transcoding.

We can feature-flag option 2 under omit_google_protobuf_empty_responses or something.
WDYT?

[johanbrandhorst]

I think we already have a special case for google.protobuf.Empty, do we not? I wouldn't necessarily be opposed to expanding that if it's not already accurate enough. What does the return value associated with a google.protobuf.Empty look like today?

[same-id]

Same as just message AnotherEmpty {}

[johanbrandhorst]

I guess that's actually accurate per the behavior of the JSON marshaler, so maybe there's not much we can do there.

[same-id]

Yea

To be honest I'm also not so sure about if it's okay to document that an API call does not return content, when in practice it does, this is a limitation of gRPC transcoding.

It's just for optics mainly