Resolve cyclic dependencies by aligning GRPCClient derived classes and NodeConnection to StartStop pattern

[CMCDragonkai]

Specification

Currently we have 2 connection abstractions in PK:

1. network - Connection and ConnectionForward and ConnectionReverse - uses StartStop
2. grpc & nodes - GRPCClient, GRPCClientAgent, GRPCClientClient and NodeConnection - uses CreateDestroy

As you can see the network is using the StartStop pattern and the grpc & nodes is using CreateDestroy.

The CreateDestroy is fundamentally more limiting than StartStop, as it enforces a sort of logical order, where asynchronous side-effects must occur first before you are given the actual object instance. Thus tying the object-lifetime to asynchronous setup and shutdown.

Originally I wanted to apply CreateDestroy to network domain, but was impacted by some cyclic dependencies that existed in network domain. However now there are cyclic dependencies in the grpc and nodes domain, and it appears that CreateDestroy is too limiting, and instead we need to use StartStop.

Now what are these cyclic dependencies? They all boil down to needing to do some asynchronous side-effects while having access to the instance itself.

For example in ConnectionForward, the start call makes use of this.utpSocketandthis.tlsSocket, which are all properties of the instance. Some complex refactoring could move this all into async creation, and then passed in as properties of the ConnectionForward` instance, but this is unnecessary complexity when all these properties have to be part of the instance.

In the case of NodeConnection, we have a problem where it causes us to construct the instance first new NodeConnection() and then setup the GRPCClient and then inject the client object into the NodeConnection instance.

    const nodeConnection = new NodeConnection<T>({
      host: targetHost,
      port: targetPort,
      hostname: targetHostname,
      fwdProxy: fwdProxy,
      destroyCallback,
      logger,
    });
    // ... setting up client
    // TODO: This is due to chicken or egg problem
    // see if we can move to CreateDestroyStartStop to resolve this
    nodeConnection.client = client;

The reason for this is because the creation of the GRPCClient requires the destroyCallback to be set, and that requires a reference to the nodeConnection instance.

This round-about way of doing things makes things a bit strange/convoluted for no reason.

If the creation logic of GRPCClient and NodeConnection were instead changed to StartStop pattern, then we can proceed to make this logic much more cleaner.

It does mean you may have NodeConnection instances that are not started. But that's ok, since our network proxies handle this by removing themselves from the connection proxies when they are stopped. The nodes connection make use of a generic destroyCallback to do this same behaviour. The main difference is that network connections hardcode knowledge about the connection state from the proxies, and require it during construction. See the connections parameter in ConnectionForward and ConnectionReverse. With GRPCClient, this is more difficult since nodes and grpc are separate domains, so we don't want to hardcode knowledge about nodes connection management into the grpc classes.

Additional context

• Extracting Node Connection Management out of NodeManager to NodeConnectionManager #310 - PR that extracted node connection management into NodeConnectionManager
• Allow generic extensions to Id js-id#13 (comment) - Some discussion about polymorphic this, this may not be relevant if we use StartStop

Tasks

1. Change GRPCClient and GRPCClientClient, GRPCClientAgent and GRPCClientTest to StartStop
2. Change NodeConnection to StartStop
3. Adapt the NodeConnectionManager, and base it on how ForwardProxy and ReverseProxy handles it
4. Update all tests

[CMCDragonkai]

When this issue is being fixed, change the host and port parameters for GRPCClient and GRPCServer back to getHost() and getPort() to match network domain API. These were meant to be methods as they get changed during running of the class instance.

[CMCDragonkai]

Not relevant for GRPCClient because we are now using RPCClient.

The RPCClient only requires stream factory callback. It does not require something that requires the NodeConnection.

Therefore once integrated the order of operations is:

1. Transport layer
2. RPCClient
3. NodeConnection

The NodeConnectionManager would have the responsibility of creating node connections that encapsulate a 1-to-1 rpc client, which itself uses the stream factory callback that would be acquired somehow from the transport layer. The transport layer would be QUIC for node connection.

[CMCDragonkai]

This should be resolved by the agent and client RPC migration @tegefaulkes. Can you check this into that issue too.

---

Actually already closed DW.