Need best practices for constructing GRPC clients documented or in samples

[davidkel]

Using just node as a client for now I messed around with changing the parameters to GrpcClient creation to see how helpful it was when something was not done right

1. Incorrect TLS Cert for peer in org2 when communicating to peer in org1

Error: 14 UNAVAILABLE: No connection established
    at Object.callErrorFromStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/call.js:31:26)
    at Object.onReceiveStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/client.js:179:52)
    at Object.onReceiveStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/client-interceptors.js:336:141)
    at Object.onReceiveStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/client-interceptors.js:299:181)
    at /home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/call-stream.js:145:78
    at processTicksAndRejections (internal/process/task_queues.js:79:11) {
  code: 14,
  details: 'No connection established',
  metadata: Metadata { internalRepr: Map {}, options: {} }

2. invalid hostname

Error: 14 UNAVAILABLE: Name resolution failed for target dns:localhost2:7051
    at Object.callErrorFromStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/call.js:31:26)
    at Object.onReceiveStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/client.js:179:52)
    at Object.onReceiveStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/client-interceptors.js:336:141)
    at Object.onReceiveStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/client-interceptors.js:299:181)
    at /home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/call-stream.js:145:78
    at processTicksAndRejections (internal/process/task_queues.js:79:11) {
  code: 14,
  details: 'Name resolution failed for target dns:localhost2:7051',
  metadata: Metadata { internalRepr: Map {}, options: {} }

3. invalid port

Error: 14 UNAVAILABLE: No connection established
    at Object.callErrorFromStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/call.js:31:26)
    at Object.onReceiveStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/client.js:179:52)
    at Object.onReceiveStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/client-interceptors.js:336:141)
    at Object.onReceiveStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/client-interceptors.js:299:181)
    at /home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/call-stream.js:145:78
    at processTicksAndRejections (internal/process/task_queues.js:79:11) {
  code: 14,
  details: 'No connection established',
  metadata: Metadata { internalRepr: Map {}, options: {} }

4. incorrect grpc override

Error: 14 UNAVAILABLE: No connection established
    at Object.callErrorFromStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/call.js:31:26)
    at Object.onReceiveStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/client.js:179:52)
    at Object.onReceiveStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/client-interceptors.js:336:141)
    at Object.onReceiveStatus (/home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/client-interceptors.js:299:181)
    at /home/dave/github-mine/fabric-gateway/samples/node/node_modules/@grpc/grpc-js/build/src/call-stream.js:145:78
    at processTicksAndRejections (internal/process/task_queues.js:79:11) {
  code: 14,
  details: 'No connection established',
  metadata: Metadata { internalRepr: Map {}, options: {} }

5. peer endpoint set to 'www.microsoft.com:7051' Application just hangs

It may or may not be possible to improve some of these responses as this is purely down to Grpc for node, but I think at least we should have a timeout for a grpc connection via the waitForReady api ?

[bestbeforetoday]

Timeouts on calls are not yet properly implemented in the Fabric Gateway client API, and are currently a mixed bag across different client languages, with some implementations using an arbitrary timeout and some not specifying a timeout.

Neither the client code examples at grpc.io nor (as far as I can tell) the generated gRPC client stubs make use of waitForReady on the gRPC client, so I'm not sure this is the right approach for us to be taking. Perhaps a better approach might be to allow the client application to specify gRPC call options for a service invocation, which could include a deadline at which to timeout the call.

Having a default timeout value settable for a Gateway connection might be OK, but I really want to avoid the trap of writing an abstraction of the gRPC API rather than just letting the client application make use of key elements of the gRPC API.

[bestbeforetoday]

To avoid derailing this documentation issue with implementation of timeouts, I've raised #198

[davidkel]

@bestbeforetoday Another thought that has occurred to me, we should probably discuss the keepalive options for grpc. The node-sdk users are blissfully unaware that the supplied defaults are pretty good for a lot of K8s services (but not all, eg openshift on Z). Wonder if we should include grpc keepalive values in the samples and also should definitely mention these across all the client sdks.

[mbwhite]

https://stackoverflow.com/questions/57930529/grpc-connection-use-keepalive-or-idletimeout

[mbwhite]

reference for the complexity of gRPC https://stackoverflow.com/questions/64484690/grpc-cpp-how-can-i-check-if-the-rpc-channel-connected-successfully

[mbwhite]

It's worth noting the location when the errors that Dave highlighted above; they will occur on the first actual communication to the peer.

So an application can go through the gateway.connect getNetwork getContract and it's not until the submit/evaluateTransaction that any error is reported.

I believe this can give a misleading impression of what might be wrong - the gateway.connect worked so why is it failing now? This idea then leads you down the wrong path for debugging issues.

As an aside as I was using org1-peer1.vcap.me address this makes any issue look like the port failure. Which is doubly confusing. But not the fault of gRPC.

@bestbeforetoday @andrew-coleman if there was some option for an initial 'heart-beat' on the connect it might be beneficial. in the longer term.

[bestbeforetoday]

I'm not convinced that a "heartbeat" call on the initial connect is actually helpful. Network or service outages can occur (and be resolved) at any time. The initial heartbeat may fail, but a subsequent invocation (that you can no longer attempt) may have worked. Similarly, your first invocation might succeed and your second might fail.

The confusion seems to stem from the perception that there is a single persistent connection that lives from the point you create the gRPC client connection / client / channel, or at least the point where it is passed to the Gateway connect. As described in the linked information above and the gRPC documentation, this is simply not (necessarily) the case.

[mbwhite]

@bestbeforetoday valid points; concede that a heart beat is of minimal (or none at all) use.

The observation of "the perception of.. a single persistent connection" would appear to be central thing here. Initially, I will admit that I thought it was 'different' creating the connection outside and then passing that into the Gateway. However, it's clear now exactly why that is a preferred pattern.

---

Does the naming of the functions in the SDK used by a client application though help to promote the erroneous perception?

Thinking really of that 'connect' call; is this the wrong word (with the benefit of hindsight of course).

[bestbeforetoday]

Yeah, "connect" was used to align with the API terminology of the legacy SDKs. Maybe it would have been better differently named here, but that's what we have now and I'm not keen on a breaking change to rename it.

[mbwhite]

When do version 2 of the Gateway SDKs come out :-)

Documentation though we could update.