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

Poor error message when using message in path parameter #1863

Closed
th-m opened this issue Dec 7, 2020 · 3 comments · Fixed by #1894
Closed

Poor error message when using message in path parameter #1863

th-m opened this issue Dec 7, 2020 · 3 comments · Fixed by #1894

Comments

@th-m
Copy link

th-m commented Dec 7, 2020

📚 Documentation

--grpc-gateway_out: aggregate type TYPE_MESSAGE in parameter of ScheduleAPI.BookAppointment: LocationID

rpc looks like this

 rpc BookAppointment(schedule.ScheduleRequest) returns (schedule.BookAppointmentResponse) {
    option (google.api.http) = {
      post: "/schedule/v2/{LocationID}/bookings/schedule-links"
    };
  }
import "protorepo/messages/shared/uuid.proto";

message ScheduleRequest {
  shared.UUID LocationID = 1;
  google.protobuf.Timestamp DateTime = 2;
  google.protobuf.Duration Duration = 3;
 ...
}

Given the example rpc and message I get the aforementioned error. I am unable to find any documentation on how to resolve this or what it means.

@johanbrandhorst
Copy link
Collaborator

I think the issue is the use of the type shared.UUID in a path parameter. I don't think we support this. The error message definitely needs improving, but I expect changing LocationID to string should fix your issue.

@johanbrandhorst johanbrandhorst changed the title Error Message aggregate type TYPE_MESSAGE in parameter ... unclear Poor error message when using message in path parameter Dec 7, 2020
@gunadhya
Copy link
Contributor

Hi @johanbrandhorst, I'm new to this project and I'd like to work on this issue. I think the error is framed on this line

return Parameter{}, fmt.Errorf("aggregate type %s in parameter of %s.%s: %s", target.Type, meth.Service.GetName(), meth.GetName(), path)

Could you help me figure out a better format?

@johanbrandhorst
Copy link
Collaborator

Hi @gunadhya, yes that seems like the place. I think for users that encounter this error we should make it clear that they can't use that protobuf type for a path parameter, and also maybe suggest a fix? How about

fmt.Errorf("%s.%s: %s is a protobuf message type. Protobuf message types cannot be used as path parameters, use a scalar value type (such as string) instead". meth.Service.GetName(), meth.GetName(), path)

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

Successfully merging a pull request may close this issue.

3 participants