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

proto: no slice oenc for *reflect.rtype = []*reflect.rtype #551

Closed
srenatus opened this issue Feb 20, 2018 · 3 comments
Closed

proto: no slice oenc for *reflect.rtype = []*reflect.rtype #551

srenatus opened this issue Feb 20, 2018 · 3 comments

Comments

@srenatus
Copy link
Contributor

Hey there

With the introduction of error details in JSON responses, #515, we've also gotten us an extra line of output for every JSON error encoding process:

proto: no slice oenc for *reflect.rtype = []*reflect.rtype

...irregardless of whether this error contains error details or not. Looking back at the PR's CI run, it has been in there right from the start.

The line comes from proto.(*Properies).setEncAndDec, called via jsonpb.jsonProperties.

However, since the tests pass, it seems like whatever is happening there wouldn't have to happen?

Have you got any idea how to get rid of that?

(While looking into this, I've also started wondering why grpc-gateway's error details don't have @type fields, but that's a different matter. But would you think they should have them?)

@achew22
Copy link
Collaborator

achew22 commented Feb 20, 2018

Is this log output, or is it included in the actual output of the server?

@srenatus
Copy link
Contributor Author

It goes to the server's stderr here.

@itizir
Copy link

itizir commented Feb 23, 2018

Hi! Oh yeah, was wondering about this the other day too... Nice one with Any, but it's still not fixing it because it should be pointers, not values! :)

Also the protobuf tag would need to contain rep, since it's a slice, for it to work properly.

But actually, if errorBody needs to be a proto.Message, why don't you define it through the protobuf generator?

syntax = "proto3";

import "google/protobuf/any.proto";

message ErrorBody {
    string error = 1;
    int32 code = 2;
    repeated google.protobuf.Any details = 3;
}

And then by the way it would indeed be nice to unify with the stream errors. :)
#538

srenatus added a commit to srenatus/grpc-gateway that referenced this issue Feb 24, 2018
As advised by @itizir in the discussion of grpc-ecosystem#551.

However, I've gone with the minimal change to make the error go away, I
haven't introduced a proper, generated protobuf definition.

Also, I've noticed our error body message there looks suspiciously like
Status[1], except for the field name "Error" vs "Message".

Fixes grpc-ecosystem#551.

[1]: https://github.com/google/go-genproto/blob/2b5a72b8730b0b/googleapis/rpc/status/status.pb.go#L83-L93

Signed-off-by: Stephan Renatus <[email protected]>
achew22 pushed a commit that referenced this issue Feb 24, 2018
As advised by @itizir in the discussion of #551.

However, I've gone with the minimal change to make the error go away, I
haven't introduced a proper, generated protobuf definition.

Also, I've noticed our error body message there looks suspiciously like
Status[1], except for the field name "Error" vs "Message".

Fixes #551.

[1]: https://github.com/google/go-genproto/blob/2b5a72b8730b0b/googleapis/rpc/status/status.pb.go#L83-L93

Signed-off-by: Stephan Renatus <[email protected]>
adasari pushed a commit to adasari/grpc-gateway that referenced this issue Apr 9, 2020
As advised by @itizir in the discussion of grpc-ecosystem#551.

However, I've gone with the minimal change to make the error go away, I
haven't introduced a proper, generated protobuf definition.

Also, I've noticed our error body message there looks suspiciously like
Status[1], except for the field name "Error" vs "Message".

Fixes grpc-ecosystem#551.

[1]: https://github.com/google/go-genproto/blob/2b5a72b8730b0b/googleapis/rpc/status/status.pb.go#L83-L93

Signed-off-by: Stephan Renatus <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants