grpc / grpc-node

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

grpc-loader: Expose field options #2773

Open n0v1 opened 1 month ago

n0v1 commented 1 month ago

Is your feature request related to a problem? Please describe.

In our proto files we would like to use custom options on field level (see FieldOptions in descriptor.proto) to store application-specific data like translations, relationships or categories. We compile our proto files to file descriptor sets and load those with grpc-loader. Unfortunately custom options on field level are currently not exposed by the objects created by proto-loader.

Describe the solution you'd like

Similar to #2711 for custom options on method level we would like proto-loader to expose custom options on field level.

Would you accept a pull request for this?

murgatroid99 commented 1 month ago

This information should already be available. The objects output by proto-loader include message definition objects for each loaded message type, and those contain the full DescriptorProrot information, which contains all of the FieldDescriptorProto objects for each field, and those contain the FieldOptions. See gRFC L43 for more information about those message definition objects.

n0v1 commented 1 month ago

Hmm, strange. When I tested I got a protobufjs error. That's why I supposed it's not supported yet.

foobar.proto:

syntax = "proto3";

package dev.foobar;

import "google/protobuf/descriptor.proto";

extend google.protobuf.FieldOptions {
  optional string my_field_option = 50000;
}

message Person {
  string first_name = 1 [(my_field_option) = 'blablubb'];
  string last_name = 2;
}

message EchoObjectRequest {
  Person person = 1;
}

message EchoObjectResponse {
  Person person = 1;
}

// The foo service definition.
service FooService {
  rpc EchoObject (EchoObjectRequest) returns (EchoObjectResponse);
}

server.js:

'use strict'

const path = require('node:path')
const grpc = require('@grpc/grpc-js')
const protoLoader = require('@grpc/proto-loader')

const grpcPort = 10000

const protoPath = path.resolve(__dirname, 'foobar.proto')
const packageDefinition = protoLoader.loadSync(protoPath, {
  keepCase   : true,
  defaults   : false,
  enums      : String,
  includeDirs: [
    path.resolve(__dirname, './')
  ]
})
const protoDescriptor = grpc.loadPackageDefinition(packageDefinition)

const server = new grpc.Server()

const methodImplementations = {
  EchoObject (call, callback) {
    console.log(`EchoObject called with the following parameters:`, call.request)
    callback(null, {
      person: call.request.person,
    })
  },
}
server.addService(protoDescriptor.dev.foobar.FooService.service, methodImplementations)

console.log(`Starting Foo service`)
server.bindAsync(
  `0.0.0.0:${grpcPort}`,
  grpc.ServerCredentials.createInsecure(),
  (err) => {
    if (err) {
      throw new Error(`Could not start gRPC server on port ${grpcPort}`, err)
    }

    server.start()
    console.log(`Foo service is listening on port ${grpcPort}`)
})
$ node server.js
/home/me/git/bitbucket/proto-custom-fields/node_modules/protobufjs/ext/descriptor/index.js:488
        if ((descriptor.oneofIndex = this.parent.oneofsArray.indexOf(this.partOf)) < 0)
                                                             ^

TypeError: Cannot read properties of undefined (reading 'indexOf')
    at Field.toDescriptor (/home/me/git/bitbucket/proto-custom-fields/node_modules/protobufjs/ext/descriptor/index.js:488:62)
    at Root_toDescriptorRecursive (/home/me/git/bitbucket/proto-custom-fields/node_modules/protobufjs/ext/descriptor/index.js:142:40)
    at Root_toDescriptorRecursive (/home/me/git/bitbucket/proto-custom-fields/node_modules/protobufjs/ext/descriptor/index.js:146:13)
    at Root_toDescriptorRecursive (/home/me/git/bitbucket/proto-custom-fields/node_modules/protobufjs/ext/descriptor/index.js:146:13)
    at Root.toDescriptor (/home/me/git/bitbucket/proto-custom-fields/node_modules/protobufjs/ext/descriptor/index.js:121:5)
    at createPackageDefinition (/home/me/git/bitbucket/proto-custom-fields/node_modules/@grpc/proto-loader/build/src/index.js:176:33)
    at Object.loadSync (/home/me/git/bitbucket/proto-custom-fields/node_modules/@grpc/proto-loader/build/src/index.js:223:12)
    at Object.<anonymous> (/home/me/git/bitbucket/proto-custom-fields/server.js:12:39)
    at Module._compile (node:internal/modules/cjs/loader:1376:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1435:10)

Node.js v20.11.1

(using @grpc/grpc-js 1.10.9 and @grpc/proto-loader 0.7.13, indirect dependency protobufjs 7.3.0)


Update: I don't get this error when compiling a descriptor set with --include_imports option (protoc --proto_path=./ --descriptor_set_out=./descriptor_set.pb --include_imports foobar.proto) and loading that:

const protoDescriptorSet = fs.readFileSync(path.resolve(__dirname, 'descriptor_set.pb'))
const packageDefinition = protoLoader.loadFileDescriptorSetFromBuffer(protoDescriptorSet, {
  keepCase   : true,
  defaults   : false,
  enums      : String,
  includeDirs: [
    path.resolve(__dirname, './')
  ]
})

However I don't find the value for custom field option my_field_option anywhere in packageDefinition. Would expect to see in on dev.foobar.Person.type.field[0].options but it's null:

{
  name: "first_name",
  extendee: "",
  number: 1,
  label: "LABEL_OPTIONAL",
  type: "TYPE_STRING",
  typeName: "",
  defaultValue: "",
  options: null,
  oneofIndex: 0,
  jsonName: "",
}

Might be related to https://github.com/protobufjs/protobuf.js/issues/1072.

murgatroid99 commented 1 month ago

@grpc/proto-loader just passes along the descriptors produced by Protobuf.js. It looks like Protobuf.js isn't successfully loading field option information into the descriptors, and that is a bug that needs to be fixed in Protobuf.js itself.