grpc / grpc-node

gRPC for Node.js
https://grpc.io
Apache License 2.0
4.43k stars 640 forks source link

Unable to use google.protobuf.Struct with @grpc/grpc-js #2747

Open Zoddo opened 3 months ago

Zoddo commented 3 months ago

Problem description

When trying to respond with a google.protobuf.Struct, the call errors with status 13 INTERNAL Error serializing response: Expected argument of type google.protobuf.Struct.

Reproduction steps

Use this proto file:

syntax = "proto3";
import "google/protobuf/struct.proto";

package test;

message Empty {}

service Test {
  rpc getStruct(Empty) returns (google.protobuf.Struct) {}
}

Compile it with:

node_modules/.bin/grpc_tools_node_protoc \
  --grpc_out="grpc_js:./" \
  --js_out="import_style=commonjs,binary:./" \
  test.proto

Then create the following server:

const grpc = require('@grpc/grpc-js');
const { Struct } = require('google-protobuf/google/protobuf/struct_pb');
const { TestService } = require('./test_grpc_pb');

const server = new grpc.Server();
server.addService(TestService, {
  getStruct: (call, callback) => {
    callback(null, Struct.fromJavaScript({ test: 1, b: 'c' }));
  },
});

server.bindAsync('0.0.0.0:4000', grpc.ServerCredentials.createInsecure());

Run it, and try to call getStruct (using Postman, for example). The server replies with an error: Error serializing response: Expected argument of type google.protobuf.Struct

I also tried with a plain object:

callback(null, {                                                                                                                                                                        
  fields: {                                                                                                                                                              
    test: { kind: 'numberValue', numberValue: 1 },                                                                                                                       
    b: { kind: 'stringValue', stringValue: 'c' }                                                                                                                         
  }
});

I get the same error.

Maybe I'm missing something obvious, but I've been trying a lot of things for the past hours and I still can't figure how to send a Struct...

Environment

Zoddo commented 3 months ago

As we say, things are clearer in the morning! I found out what's my issue.

This error is coming from this check in the compiled proto file:

if (!(arg instanceof google_protobuf_struct_pb.Struct)) {
  throw new Error('Expected argument of type google.protobuf.Struct');
}

Now, why this cause issues in my case? In my project, compiled proto files are put in a separate package that is installed with npm (to prevent code duplication).

Basically:

my repo
|─ server
|   |─ package.json
|   `─ server.js
`─ lib/testproto
    |─ package.json
    |─ test.proto
    |─ test_grpc_pb.js
    `─ test_pb.js

So when in server, I npm I -S ../lib/testproto, npm generates a symlink like this:
node_modules/testproto -> ../../lib/cache

Because it's a symlink, module deduplication between server and testproto doesn't work, as shown by npm ls google-protobuf in server:

zoddo@arch:~/git/myproject/server$ npm ls google-protobuf
server@1.0.0 /home/zoddo/git/myproject/server
├─┬ testproto@1.0.0 -> ./../lib/testproto
│ ├── google-protobuf@3.21.2
│ └─┬ ts-protoc-gen@0.15.0
│   └── google-protobuf@3.21.2 deduped
└── google-protobuf@3.21.2

As such, compiled files in testproto don't use the same files than server when doing require('google-protobuf'), breaking the instanceof check done in the compiled proto file.

In other words, that means that in the server, I generated an instance of Struct coming from:

server/node_modules/google-protobuf/google/protobuf/struct_pb.js

...while in testproto, it is checking if it's an instance of Struct coming from:

lib/cache/node_modules/google-protobuf/google/protobuf/struct_pb.js

(while it's the same code in the 2 files, they are different objects for the JS engine)

I'm not sure how to properly fix this issue. We can't even check arg.constructor.name because Struct isn't a class, but an anonymous function.

Right now I have a workaround by using TestService.getStruct.responseType.fromJavaScript(myObj) instead of Struct.fromJavaScript(myObj), but that's a hacky solution I don't like at all!