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

grpc-loader: Expose field options #2773

Open
n0v1 opened this issue Jun 10, 2024 · 3 comments
Open

grpc-loader: Expose field options #2773

n0v1 opened this issue Jun 10, 2024 · 3 comments

Comments

@n0v1
Copy link
Contributor

n0v1 commented Jun 10, 2024

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
Copy link
Member

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
Copy link
Contributor Author

n0v1 commented Jun 11, 2024

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 protobufjs/protobuf.js#1072.

@murgatroid99
Copy link
Member

@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.

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

No branches or pull requests

2 participants