timostamm / protobuf-ts

Protobuf and RPC for TypeScript
Apache License 2.0
1.05k stars 127 forks source link

Add ability to set grpc-js MetadataOptions when using GrpcTransport #259

Open kskalski opened 2 years ago

kskalski commented 2 years ago

When grpc-js calls are made, it's possible to set Metadata, which besides key-value pairs, also contains a few pre-defined options set in constructor with MetadataOptions object ).

Currently the generated rpc clients in protobuf-ts take RpcOptions object, which contains meta: RpcMetadata, but this option doesn't seem to support populating the same options as in MetadataOptions when call is made using GrpcTransport.

I suppose GrpcMetadata object could be added to grpc-transport package, which would extend RpcMetadata and calls using grpc transport could translate them to fully populate Metadata. I'm not sure if this translation could be done in a type-safe way - this would probably require baking in some extensibility into generic client.

kskalski commented 2 years ago

I suppose something like this could be added to GrpcCallOptions

    /**
     * This option can be provided when calling a client method.
     * The MetadataOptions are used for creating Metadata object passed
     * to request factory method of the @grpc/grpc-js client as the
     * "metadata" argument.
     */
     metadataOptions?: MetadataOptions;

and construction of metadata would done by util method, e.g.

export function metadataToGrpc(from: RpcMetadata, options?: MetadataOptions, idempotentMethod?: boolean):
 grpc.Metadata {
...
}

One obstacle is that currently grpc-js doesn't expose interface MetadataOptions (I've sent a PR to add it, let's see if it goes through).

Also a question arises how options should be merged with method's idempotency - my impression is that it should be used as default if options are not provided or they do not contain explicit idempotentRequest value.

timostamm commented 2 years ago

Yes, I agree with adding metadataOptions?: MetadataOptions to GrpcCallOptions. Thanks for opening the PR.

kskalski commented 2 years ago

This PR https://github.com/timostamm/protobuf-ts/pull/261 should allow the options to be passed by user

Janaka-Steph commented 2 years ago

Does grpcweb-transport affected by the same issue?

kskalski commented 2 years ago

grpcweb-transport doesn't use grpc-js, so it doesn't consume its MetadataOptions. One could imagine some of the functionalities from those options to be implemented in grpcweb-transport, but that is an independent feature.