NULL deserialization of Oneof when using GRPC

[buckhx]

A message with a oneof is not properly serializing using GRPC.

Here's an example of the a json-encoded message payload before it is written and then after is is read. I encountered this having a client send a message to a GRPC method and the oneof be null on the server.

WRITE: {
	"label": "test",
	"contact": {
		"email": "[email protected]"
	},
	"created": {
		"seconds": 1481225819,
		"nanos": 422943804
	},
	"Entity": {
		"Person": {
			"name": {
				"first": "test",
				"last": "user"
			}
		}
	}
}

READ: {
	"label": "test",
	"contact": {
		"email": "[email protected]"
	},
	"created": {
		"seconds": 1481225819,
		"nanos": 422943804
	},
	"Entity": null
}

I'm sure this could be replicated w/out GRPC for easy debugging.

Switching to golang/protobuf has proper oneof serialization

[awalterschulze]

It would be great if you could reproduce this without grpc?

[buckhx]

My initial tests seem to work fine. I'll add in pieces that are being used outside of GRPC (WKT, annotations..) to a test repo to reflect the production case I ran into as I go and reach out when I find something. Haven't looked into grpc's proto marshaling, but I'm assuming the use "github.com/golang/protobuf/proto".

Here's a repo w/ the tests https://github.com/buckhx/gogo-test

[awalterschulze]

wow nice effort. Thank you. Give me a few minutes.

[awalterschulze]

I just cloned it and ran the test and it is passing, is this intended?

[awalterschulze]

Also note I just merge alot of updates from golang/protobuf

[buckhx]

Yeah those tests passed, but it's not an exact replica of my environment in the project that is getting the nulls so I'll add in a few more things and see if they break these tests. Also I'll try your updates and see if I'm still getting nulls in my other project. The only funky thing I noticed in these tests is that the json serialization is CamelCase on the oneofs and lower_snake on the other fields.

…

On Sat, Dec 10, 2016, 1:26 PM Walter Schulze ***@***.***> wrote: Also note I just merge alot of updates from golang/protobuf — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#238 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AATNZx_K41kGZUgtw12Fjn32wH_S-NE1ks5rGu7GgaJpZM4LINox> .

[awalterschulze]

Ok wait I think I am seeing a problem in your code.
You should use the jsonpb package to serialize your json.

[awalterschulze]

Also this might be interesting for you, since you are using grpc and json
#178

[buckhx]

Good call on the jsonpb. I was just using it for debugging, but will be using grpc-gateway so that's a good data point. I pulled out the json parts to avoid an red herrings.

The tests now break in https://github.com/buckhx/gogo-test w/ what I am seeing in my original grpc project.

Using "github.com/golang/protobuf/proto" w/ a gogo proto that includes a oneof seems to be the issue. Not sure if there's a mechanism that can be hooked into to pick the serializer for grpc as was suggested in the jsonpb thread or not.

Here's the test log using "github.com/golang/protobuf/proto"

--- FAIL: TestGogoRoundTrip (0.00s)
	serialize_test.go:34: label:"ash" contact:<email:"[email protected]" > person:<name:<first:"ash" last:"ketchum" > > 
	serialize_test.go:43: label:"ash" contact:<email:"[email protected]" > 
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x6f8ff]

goroutine 17 [running]:
panic(0x147920, 0xc42000a0b0)
	/Users/buck/.gvm/gos/go1.7.3/src/runtime/panic.go:500 +0x1a1
testing.tRunner.func1(0xc4200a0180)
	/Users/buck/.gvm/gos/go1.7.3/src/testing/testing.go:579 +0x25d
panic(0x147920, 0xc42000a0b0)
	/Users/buck/.gvm/gos/go1.7.3/src/runtime/panic.go:458 +0x243
github.com/buckhx/gogo-test_test.TestGogoRoundTrip(0xc4200a0180)
	/Users/buck/.gvm/pkgsets/go1.7.3/global/src/github.com/buckhx/gogo-test/serialize_test.go:49 +0x6cf
testing.tRunner(0xc4200a0180, 0x182380)
	/Users/buck/.gvm/gos/go1.7.3/src/testing/testing.go:610 +0x81
created by testing.(*T).Run
	/Users/buck/.gvm/gos/go1.7.3/src/testing/testing.go:646 +0x2ec
FAIL	github.com/buckhx/gogo-test	0.008s
?   	github.com/buckhx/gogo-test/gen/gogo	[no test files]
?   	github.com/buckhx/gogo-test/gen/golang	[no test files]
?   	github.com/buckhx/gogo-test/vendor/github.com/gogo/protobuf/proto	[no test files]
?   	github.com/buckhx/gogo-test/vendor/github.com/golang/protobuf/proto	[no test files]

[awalterschulze]

grpc has a WithCodec method that might be useful when used with this
https://github.com/gogo/protobuf/blob/master/codec/codec.go
Although I have been told its not thread safe
#205

You are not supposed to serialize a gogoprotobuf generated proto message with golang/protobuf.
You should always use the appropriate library.

Except when using gogoproto_import=false, because then the generated code will still using the golang/protobuf library.
This is what gofast_out uses internally.

[awalterschulze]

I assume my answer fixed this issue, otherwise please reopen