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

gogoproto.stdtime = true is causing a panic error of interface conversion: *time.Time is not proto.Message: missing method ProtoMessage #212

Closed
liranp opened this issue Oct 13, 2016 · 19 comments
Labels

Comments

@liranp
Copy link

liranp commented Oct 13, 2016

I'm having a similar issue as reported in #209. I'm getting the following runtime (panic) error using [(gogoproto.stdtime) = true]:

proto: no coders for time.Time
2016/10/13 22:41:46 http: panic serving [::1]:49533: interface conversion: *time.Time is not proto.Message: missing method ProtoMessage
goroutine 55 [running]:
net/http.(*conn).serve.func1(0xc42013c000)
    /usr/local/Cellar/go/1.7.1/libexec/src/net/http/server.go:1491 +0x12a
panic(0x56b9e0, 0xc420012c00)
    /usr/local/Cellar/go/1.7.1/libexec/src/runtime/panic.go:458 +0x243
github.com/golang/protobuf/jsonpb.(*Marshaler).marshalValue(0x8b0c40, 0xc4201ff678, 0xc4200e28c0, 0x5fb080, 0xc4204ee0a0, 0x199, 0x0, 0x0, 0xc4201fe148, 0xc4200e28c0)
    /Users/liran/Development/gocode/src/github.com/golang/protobuf/jsonpb/jsonpb.go:443 +0xaeb
github.com/golang/protobuf/jsonpb.(*Marshaler).marshalField(0x8b0c40, 0xc4201ff678, 0xc4200e28c0, 0x5fb080, 0xc4204ee0a0, 0x199, 0x0, 0x0, 0x0, 0xc420122168)
    /Users/liran/Development/gocode/src/github.com/golang/protobuf/jsonpb/jsonpb.go:366 +0x10b
github.com/golang/protobuf/jsonpb.(*Marshaler).marshalObject(0x8b0c40, 0xc4201ff678, 0xd9a5d0, 0xc4204ee0a0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
    /Users/liran/Development/gocode/src/github.com/golang/protobuf/jsonpb/jsonpb.go:225 +0x484
github.com/golang/protobuf/jsonpb.(*Marshaler).marshalValue(0x8b0c40, 0xc4201ff678, 0xc4200e23c0, 0x5a0a00, 0xc4204ee0a0, 0x199, 0x0, 0x0, 0x602b43, 0x1)
    /Users/liran/Development/gocode/src/github.com/golang/protobuf/jsonpb/jsonpb.go:443 +0xb8e
github.com/golang/protobuf/jsonpb.(*Marshaler).marshalField(0x8b0c40, 0xc4201ff678, 0xc4200e23c0, 0x5a0a00, 0xc4204ee0a0, 0x199, 0x0, 0x0, 0x10, 0xc4201220f8)
    /Users/liran/Development/gocode/src/github.com/golang/protobuf/jsonpb/jsonpb.go:366 +0x10b
github.com/golang/protobuf/jsonpb.(*Marshaler).marshalObject(0x8b0c40, 0xc4201ff678, 0xd9a520, 0xc4204ee090, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
    /Users/liran/Development/gocode/src/github.com/golang/protobuf/jsonpb/jsonpb.go:225 +0x484
github.com/golang/protobuf/jsonpb.(*Marshaler).marshalValue(0x8b0c40, 0xc4201ff678, 0xc4200e2000, 0x5d96e0, 0xc42045a158, 0x196, 0x0, 0x0, 0xc4201ff0a8, 0xc4200e2000)
    /Users/liran/Development/gocode/src/github.com/golang/protobuf/jsonpb/jsonpb.go:443 +0xb8e
github.com/golang/protobuf/jsonpb.(*Marshaler).marshalField(0x8b0c40, 0xc4201ff678, 0xc4200e2000, 0x5d96e0, 0xc42045a158, 0x196, 0x0, 0x0, 0x0, 0xc4201220a0)
    /Users/liran/Development/gocode/src/github.com/golang/protobuf/jsonpb/jsonpb.go:366 +0x10b
github.com/golang/protobuf/jsonpb.(*Marshaler).marshalObject(0x8b0c40, 0xc4201ff678, 0x895680, 0xc42045a158, 0x0, 0x0, 0x0, 0x0, 0xc4201ff698, 0xecda)
    /Users/liran/Development/gocode/src/github.com/golang/protobuf/jsonpb/jsonpb.go:225 +0x484
github.com/golang/protobuf/jsonpb.(*Marshaler).Marshal(0x8b0c40, 0x88dd40, 0xc42011a000, 0x895680, 0xc42045a158, 0x11218, 0x70)
    /Users/liran/Development/gocode/src/github.com/golang/protobuf/jsonpb/jsonpb.go:78 +0xca
github.com/grpc-ecosystem/grpc-gateway/runtime.(*JSONPb).marshalTo(0x8b0c40, 0x88dd40, 0xc42011a000, 0x5c7c40, 0xc42045a158, 0x609d20, 0xc)
    /Users/liran/Development/gocode/src/github.com/grpc-ecosystem/grpc-gateway/runtime/marshal_jsonpb.go:49 +0xa3
github.com/grpc-ecosystem/grpc-gateway/runtime.(*JSONPb).Marshal(0x8b0c40, 0x5c7c40, 0xc42045a158, 0xc42045a158, 0x895680, 0xc42045a158, 0x8dcf68, 0x0)
    /Users/liran/Development/gocode/src/github.com/grpc-ecosystem/grpc-gateway/runtime/marshal_jsonpb.go:33 +0xc9

It is worth mentioning that with the standard timestamp.Timestamp from golang/protobuf it works fine (even through grpc-gateway).

Attached a gist with the relevant files.

Any help will be much appreciated!

@atombender
Copy link
Contributor

atombender commented Oct 14, 2016

I believe (as we discussed on #grpc in the Gophers Slack) that this is caused by mixing grpc-gateway with Gogo.

grpc-gateway uses jsonpb from golang/protobuf, which doesn't seem to support time.Time, to marshal the data. It panics here:

    if v.Kind() == reflect.Struct {
        return m.marshalObject(out, v.Addr().Interface().(proto.Message), indent+m.Indent, "")
    }

It's assuming that any struct it finds is marshalable as a proto.Message, which is not the case for time.Time.

Gogo has its own fork of jsonpb, which explicitly supports time.Time.

So the problem at the moment is that grpc-gateway can't be used with Gogo. I don't think you can tell grpc-gateway to use a different JSON marshaler, either. From what I can tell, it's not possible.

@atombender
Copy link
Contributor

Upon further investigation, you can use this marshaler from CockroachDB, which can plug into grpc-gateway like so:

m := new(protoutil.JSONPb)
runtime.NewServeMux(runtime.WithMarshalerOption(runtime.MIMEWildcard, m))

@awalterschulze
Copy link
Member

Should this issue stay open?

@atombender
Copy link
Contributor

Pretty sure I addressed it.

@awalterschulze
Copy link
Member

I agree :)

Maybe we could ask @tamird to factor this out into something that everyone can use?
And make it a package under github.com/gogo/jsonpb_gateway or something like that.

@tamird
Copy link
Contributor

tamird commented Oct 14, 2016

Wasn't there an issue in https://github.com/grpc-ecosystem/grpc-gateway that tracked such an extraction?

@liranp
Copy link
Author

liranp commented Oct 16, 2016

@atombender, @awalterschulze: It sill doesn't work well for me, even with the util.JSONPb marshaler from cockroachdb/cockroach. Well, there's no panic now, but I'm getting the following warning messages:

proto: no encoder for sec int64 [GetProperties]
proto: no encoder for nsec int32 [GetProperties]
proto: no encoder for loc *time.Location [GetProperties]

And yet, the timestamp itself doesn't marshaled into JSON as a valid timestamp:

{
    "value": {
        "timestamp": "0001-01-01T00:00:00.000Z"
    }
}

Attached an updated gist.

@liranp
Copy link
Author

liranp commented Oct 17, 2016

@awalterschulze Based on @atombender's example, adding these options to my .proto file solved the issue:

option (gogoproto.sizer_all) = true;
option (gogoproto.marshaler_all) = true;
option (gogoproto.unmarshaler_all) = true;

The issue occurs again as soon as they are defined with a false value (which is the default, cause I never declared them before).

@awalterschulze
Copy link
Member

Thats is quite interesting. I'll have to make a test case for that and check it. I assume it is call GetProperties in generator.go and when the Marshal() method is generated it skips this call. I don't know what the fix is yet, but since the json is marshaled correctly, I assume this is low priority.

@awalterschulze
Copy link
Member

@tamird ^^ doesn't seem like the issue was ever opened

@awalterschulze
Copy link
Member

I have changed test/types/types.proto to have a sizer_all=false
Then I ran make regenerate this regenerates all combinations of marshaler and unmarshaler true/false, etc.
I then went into the neither folder where sizer, marshaler and unmarshaler are now set to false.
There is a generated test using the jsonpb library to marshal and unmarshal

message StdTypes {
  google.protobuf.Timestamp nullableTimestamp = 1 [(gogoproto.stdtime) = true];
  google.protobuf.Duration nullableDuration = 2 [(gogoproto.stdduration) = true];
  google.protobuf.Timestamp timestamp = 3 [(gogoproto.stdtime) = true, (gogoproto.nullable) = false];
  google.protobuf.Duration duration = 4 [(gogoproto.stdduration) = true, (gogoproto.nullable) = false];
}

Which also gives me a pointer to time.Time in the resulting struct.
I do not see the warning messages.
Are you sure grpc-gateway is not still calling golang/protobuf/jsonpb?

@liranp
Copy link
Author

liranp commented Oct 22, 2016

@awalterschulze It works well for me since I changed the marshaler used by grpc-gateway (as we discussed on #216). Sorry that I haven't updated here, I was a bit busy the last few days.

@awalterschulze
Copy link
Member

Ok cool. So we can close this issue again?

@liranp
Copy link
Author

liranp commented Oct 23, 2016

@awalterschulze Pretty sure we can.

@awalterschulze
Copy link
Member

Sweet Thank you

@Mistobaan
Copy link

I just hit this issue again. Is the cockroach jsonpb module still the best way to address this problem?

@awalterschulze
Copy link
Member

Looks like it.
You are welcome to open an issue on grpc-gateway, but I think we have done as much as we can.
Except if you have another idea?

@johanbrandhorst
Copy link
Member

@Mistobaan FYI, there's now an "official" package dealing with this: https://github.com/gogo/gateway. See https://github.com/gogo/grpc-example for an example of this in use.

@erizaver
Copy link

erizaver commented Aug 12, 2019

3 days of researching and i found a solution. Just in case someone having this issue and can`t find an answer try adding this to proto file
option (gogoproto.goproto_registration) = true; option (gogoproto.gogoproto_import) = true;

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

No branches or pull requests

7 participants