"proto: Marshal called with nil" after 1.34.0 upgrade

[alexshtin]

What version of gRPC are you using?

1.34.0

What did you do?

After updating to 1.34.0 we started to get proto: Marshal called with nil when gRPC method is called and nil is passed as request parameter. I believe it is because of this change. I agree, it is wrong, and I am currently fixing our code base but this is sort of breaking change and needs to be called out in release notes at least.

[easwars]

Ideally we would like to fix this. But looking at the code, it is not very clear to us how it did not panic earlier. We would like to understand better. Were you passing a nil value which implemented the proto.Marshaler interface?

[alexshtin]

No, it was just pure nil but when nil is assigned to typed param which then passed to interface{} param it becomes interface with type and nil value. Similar to: https://play.golang.org/p/sDJBWQweAYw

Actually I found only case in our code and it is fixed here: temporalio/temporal#1067 (wfClient is gRPC generated client. Btw, we use gogo, but I think it doesn't matter as it calls standart grpc lib anyway).

[stale]

This issue is labeled as requiring an update from the reporter, and no update has been received after 6 days. If no update is provided in the next 7 days, this issue will be automatically closed.

[alexshtin]

I did my best. Please let me know if I can provide more info.

[dfawley]

Btw, we use gogo, but I think it doesn't matter as it calls standart grpc lib anyway

With the standard protobuf library, we always got panics when passing nil messages:

$ git checkout upstream/v1.20.x
<....>
$ git diff
...
--- a/test/end2end_test.go
+++ b/test/end2end_test.go
@@ -955,7 +955,7 @@ func (s) TestContextDeadlineNotIgnored(t *testing.T) {
-       if _, err := tc.EmptyCall(context.Background(), &testpb.Empty{}); err != nil {
+       if _, err := tc.EmptyCall(context.Background(), nil); err != nil {
...
$ go test -run "Test/ContextDeadlineNotIgnored" ./test/...
--- FAIL: Test (0.06s)
    --- FAIL: Test/ContextDeadlineNotIgnored (0.06s)
        end2end_test.go:578: Running test in no-balancer environment...
        end2end_test.go:959: TestService/EmptyCall(_, _) = _, rpc error: code = Internal desc = grpc: error while marshaling: proto: Marshal called with nil, want _, <nil>

Seems like this was only a change for gogoproto users using nil messages. As mentioned in https://github.com/grpc/grpc-go/pull/3958/files#r571175706:

gogoproto is not officially supported by gRPC-Go.

If you want a codec that suits your needs better, it's easy to install your own. Just copy/paste this code into another package, update the parts that are important, and import it in your main package after any grpc imports. (If you don't control main, you can give it a new name and use the Codec Call-/Dial-/Server- Options.)