Deadline examples/docs could use clarification about grpc-node

[murgatroid99]

How are you getting this result? When I run the examples I don't see that behavior, I see the intended behavior of setting the deadline and having requests end with the DEADLINE_EXCEEDED status.

[jhump]

@murgatroid99, I'll have to dig through the commit history to find the commit where it was failing. The working code looks like so:

const call = client.unary(
      uReq,
      metadata,
      callOptions,
      (err: ServiceError | null, response: UnaryResponse) => {
          // ...
      }
    );

Previously, based on the docs indicating a deadline in the second parameter, I think the second paramter was previously a combined object with both request metadata and a deadline key. It's unclear from the docs that wouldn't work. And when there was no other request metadata, it would have basically looked like exactly like the example.

@smaye81, can you help me unearth a better snippet that repros this?

[murgatroid99]

Just to clarify, the handler does inspect the arguments to dynamically decide how to interpret the positional arguments. It is unambiguous because metadata must be an instance of the Metadata class, callOptions must be an object, and the callback argument must be a function.

[jhump]

Ah, I see. I still think the docs/examples could use an update to clarify this. I think our code may have been putting the deadline into a Metadata class, since it wasn't immediately obvious from the examples that is wrong.

[murgatroid99]

I'm not sure what you mean about putting the deadline into a Metadata class. That's clearly not what the example shows.

For clarification, I think the examples collectively cover the different cases. The hello world example demonstrates making a request without providing either metadata or callOptions. The deadline example demonstrates making a request with callOptions but not metadata. The metadata example demonstrates making a request with metadata but not callOptions. And you seem to have figured out how to send both from other information.

[jhump]

Looks like the issues arise from using TypeScript. The .ts.d file generated has signatures like so:

public unary(request: test.UnaryRequest, callback: (error: grpc.ServiceError | null, response: test.UnaryResponse) => void): grpc.ClientUnaryCall;

public unary(request: test.UnaryRequest, metadata: grpc.Metadata, callback: (error: grpc.ServiceError | null, response: test.UnaryResponse) => void): grpc.ClientUnaryCall;

public unary(request: test.UnaryRequest, metadata: grpc.Metadata, options: Partial<grpc.CallOptions>, callback: (error: grpc.ServiceError | null, response: test.UnaryResponse) => void): grpc.ClientUnaryCall;

None of those signatures accepts a CallOptions as the second parameter, but the examples clearly show adding the deadline to the second parameter. I think the TS generator needs to also emit another signature:

public unary(request: test.UnaryRequest, options: Partial<grpc.CallOptions>, callback: (error: grpc.ServiceError | null, response: test.UnaryResponse) => void): grpc.ClientUnaryCall;

[murgatroid99]

Which code generator is producing those TypeScript files?

[smaye81]

Actually in looking at how we are generating the d.ts files, it is using grpc_tools_node_protoc_ts, which isn't a part of grpc-js. I assumed that the grpc_tools prefix meant it was part of the grpc-tools package. We will follow up with that plugin author and we can close this issue.