Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when using oneof name in response body selector #1264

Open
jefferai opened this issue May 1, 2020 · 8 comments
Open

Error when using oneof name in response body selector #1264

jefferai opened this issue May 1, 2020 · 8 comments

Comments

@jefferai
Copy link
Contributor

jefferai commented May 1, 2020

As asked from my question in #585 I'm filing a new bug here.

I'm trying to have the response_body in the HTTP options use the oneof field, so that we can have alternate schemas in the response. However, this causes an error. Here is a minimal proto:

syntax = "proto3";

package services;

option go_package = "services;services";

import "google/api/annotations.proto";

service HostService {
  // This retrieves the host specified in the request using the basic view.
  rpc GetHost(GetHostRequest) returns (GetHostResponse) {
    option (google.api.http) = {
      get: "/hosts/{id}"
      response_body: "item"
    };
  }
}

message HostA {
  string foo = 1;
}

message HostB {
  string bar = 1;
}

message GetHostRequest {
  string id = 1;
}

message GetHostResponse {
  oneof item {
    HostA a = 1;
    HostB b = 2;
  }
}

Here is tree output:

.
├── gen
│   └── service
└── service
    ├── service.proto
    └── third_party
        ├── google
        │   └── api
        │       ├── annotations.proto
        │       ├── http.proto
        │       └── httpbody.proto
        └── protoc-gen-swagger
            └── options
                ├── annotations.proto
                └── openapiv2.proto

And here is my invocation:
protoc -I. -I service/third_party --go_out=plugins=grpc,paths=source_relative:./gen --grpc-gateway_out=logtostderr=true,paths=source_relative:./gen service/service.proto

When I run the command I get:
--grpc-gateway_out: no field "item" found in GetHostResponse

Note that if I change the response body def to select either "a" or "b" -- not actually what I want, but just to try it out -- I get a different error (which I think is already captured in a different ticket):
--grpc-gateway_out: 224:9: expected operand, found 'if'

@johanbrandhorst
Copy link
Collaborator

Thanks for raising the issue. The reponse_body handling is all in this file: https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-grpc-gateway/descriptor/services.go#L146. I expect we may need to add some logic to handle the case where a oneof is used. What can I do to help you bring this fix in?

@jefferai
Copy link
Contributor Author

jefferai commented May 1, 2020

Well, having never dealt with either underlying proto handling code or grpc-gateway code, any pointers you may have would be useful :-)

@jefferai
Copy link
Contributor Author

jefferai commented May 1, 2020

Closing -- it may be a bug but we're actually going to structure our API differently (for different reasons than this). If you want to keep it open to track, feel free!

@jefferai jefferai closed this as completed May 1, 2020
@johanbrandhorst
Copy link
Collaborator

Okay! Feel free to reopen again if you need it later. If you need any other support for the gateway, I'm usually available in #grpc-gateway on Gophers slack.

@jlewi
Copy link

jlewi commented Oct 12, 2021

This seems like a useful feature.

I investigated a bit and it looks like we'd want to modify the request handler template

msg, err := client.{{.Method.GetName}}(ctx, &protoReq, grpc.Header(&metadata.HeaderMD), grpc.Trailer(&metadata.TrailerMD))

To test the oneof and return the value that is actually set. When I hacked my generated code the following seemed to do the correct thing.

	msg, err := client.Get(ctx, &protoReq, grpc.Header(&metadata.HeaderMD), grpc.Trailer(&metadata.TrailerMD))

	var result proto.Message
	switch x := msg.GetResult().(type) {
	case *GetResponse_Document:
		result = x.Document
		break
	case *GetResponse_Status:
		result = x.Status
	case nil:
	default:
		result = nil;
	}

where my proto looked like

message GetResponse {  
  oneof result {
    Document document = 1;
    ResponseStatus status = 2;
  }
}

No immediate plans to submit a fix for this. We are currently evaluating the use of gRPC and gRPC-gateway. If we end up adopting it we may try to tackle this.

@johanbrandhorst
Copy link
Collaborator

Thanks for your input, I'll reopen this bug report then :).

@stale
Copy link

stale bot commented Jan 9, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jan 9, 2022
@stale stale bot closed this as completed Apr 19, 2022
@stale stale bot removed the wontfix label Apr 19, 2022
@stale
Copy link

stale bot commented Jul 10, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants