Request type validation before calling encode()

[matrosovs]

Hello,

We are having an issue sometimes, where not all fields specified in our *.proto files are present when calling service methods. When it happens, we get a cryptic exception, which doesn't tell us which exact field is missing. This makes debugging extremely hard.

We would like to add some automatic validation.

For example, let's assume this sample:

export interface SampleRequest {
  request: string;
}

export const SampleRequest = {
  encode(message: SampleRequest, writer: _m0.Writer = _m0.Writer.create()): _m0.Writer {
    if (message.request !== "") {
      writer.uint32(10).string(message.request);
    }
    return writer;
  },

  // implementations for: decode(), fromJSON(), toJSON(), create(), fromPartial()
};

<...>

export class SampleServiceClientImpl implements SampleService {
  private readonly rpc: Rpc;
  private readonly service: string;
  constructor(rpc: Rpc, opts?: { service?: string }) {
    this.service = opts?.service || SampleServiceServiceName;
    this.rpc = rpc;
    this.sampleMethod = this.sampleMethod.bind(this);
  }
  sampleMethod(request: SampleRequest): Promise<SampleResponse> {
    const data = SampleRequest.encode(request).finish();
    const promise = this.rpc.request(this.service, "SampleMethod", data);
    return promise.then((data) => SampleResponse.decode(_m0.Reader.create(data)));
  }
}

Sometimes we get a situation where SampleServiceClient.sampleMethod() is called with the parameter, which doesn't have mandatory field request. And because of it, an exception is thrown when we attempt to encode the request:

TypeError: The "string" argument must be of type string or an instance of Buffer or ArrayBuffer. Received undefined
    at Function.byteLength (node:buffer:762:11)
    at BufferWriter.write_string_buffer [as string] (/workspaces/tools-workspaces/users/grpc/node_modules/protobufjs/src/writer_buffer.js:70:27)
    at Object.encode (/workspaces/tools-workspaces/users/grpc/src/ts-proto-generated/services/sample.ts:111:25)
    at SampleServiceClientImpl.sampleMethod (/workspaces/tools-workspaces/users/grpc/src/ts-proto-generated/services/sample.ts:242:35)
    at <...>) {
  code: 'ERR_INVALID_ARG_TYPE'
}

Ideally, we are looking for the validation to happen before the line, where we encode the request:

const data = SampleRequest.encode(request).finish();

Do you have any recommendations?

[ws-gregm]

Hi @stephenh. wdyt on this matter? Should this be upgraded to an issue?

[stephenh]

Hi @matrosovs / @ws-gregm ; I agree the error message is not great.

My assumption is that most protobuf libraries (i.e. the C++/java versions) assume the type system will keep the user from creating these error states, and then just skip validation to improve performance and reduce code size.

If we wanted to have an validateRequiredFields=true type flag, that seems fine to me; if you'd like to submit a PR for that, that'd be great. Thanks!