fix: NCM getMultiConnection was failing when more than one NodeAddress

was provided
MatrixAI · Oct 16, 2023 · 5c5c7ca · 5c5c7ca
1 parent a975fd3
commit 5c5c7ca
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 22 deletions.
diff --git a/src/nodes/NodeConnectionManager.ts b/src/nodes/NodeConnectionManager.ts
@@ -852,7 +852,7 @@ class NodeConnectionManager {
         connectionsResults,
         { timer: ctx.timer, signal },
       ).finally(() => {
-        if (connectionsResults.size === resolvedAddresses.length) {
+        if (connectionsResults.size === nodeIds.length) {
           // We have found all nodes, clean up remaining connections
           abortController.abort(cleanUpReason);
         }
@@ -861,7 +861,7 @@ class NodeConnectionManager {
     // We race the connections with timeout
     try {
       this.logger.debug(`awaiting connections`);
-      await Promise.race([Promise.all(connProms)]);
+      await Promise.allSettled(connProms);
       this.logger.debug(`awaiting connections resolved`);
     } finally {
       // Cleaning up
@@ -931,8 +931,8 @@ class NodeConnectionManager {
         })
         .finally(async () => {
           if (iceProm != null) {
-            iceProm.cancel('Connection was established');
-            await iceProm;
+          iceProm.cancel('Connection was established');
+          await iceProm;
           }
         });
     // 2. if established then add to result map
@@ -1688,21 +1688,22 @@ class NodeConnectionManager {
    * The main use-case is to connect to multiple seed nodes on the same hostname.
    * @param nodeIds
    * @param addresses
-   * @param limit
    * @param ctx
    */
   public getMultiConnection(
     nodeIds: Array<NodeId>,
     addresses: Array<NodeAddress>,
-    limit?: number,
-    ctx?: Partial<ContextTimed>,
+    ctx?: Partial<ContextTimedInput>,
   ): PromiseCancellable<Array<NodeId>>;
   @ready(new nodesErrors.ErrorNodeConnectionManagerNotRunning())
-  @timedCancellable(true)
+  @timedCancellable(
+    true,
+    (nodeConnectionManager: NodeConnectionManager) =>
+      nodeConnectionManager.connectionConnectTimeoutTime,
+  )
   public async getMultiConnection(
     nodeIds: Array<NodeId>,
     addresses: Array<NodeAddress>,
-    limit: number | undefined,
     @context ctx: ContextTimed,
   ): Promise<Array<NodeId>> {
     const locks: Array<LockRequest<Lock>> = nodeIds.map((nodeId) => {

diff --git a/src/nodes/NodeManager.ts b/src/nodes/NodeManager.ts
@@ -1221,23 +1221,29 @@ class NodeManager {
       )}`,
     );
     // Establishing connections to the seed nodes
-    const connections = await this.nodeConnectionManager.getMultiConnection(
-      seedNodes,
-      filteredAddresses,
-      pingTimeoutTime,
-      { signal: ctx.signal },
-    );
+    let connections: Array<NodeId>;
+    try {
+      connections = await this.nodeConnectionManager.getMultiConnection(
+        seedNodes,
+        filteredAddresses,
+        { signal: ctx.signal },
+      );
+    } catch (e) {
+      if (
+        e instanceof nodesErrors.ErrorNodeConnectionManagerMultiConnectionFailed
+      ) {
+        // Not explicitly a failure but we do want to stop here
+        this.logger.warn(
+          'Failed to connect to any seed nodes when syncing node graph',
+        );
+        return;
+      }
+      throw e;
+    }
     logger.debug(`Multi-connection established for`);
     connections.forEach((nodeId) => {
       logger.debug(`${nodesUtils.encodeNodeId(nodeId)}`);
     });
-    if (connections.length === 0) {
-      // Not explicitly a failure but we do want to stop here
-      this.logger.warn(
-        'Failed to connect to any seed nodes when syncing node graph',
-      );
-      return;
-    }
     // Using a map to avoid duplicates
     const closestNodesAll: Map<NodeId, NodeData> = new Map();
     const localNodeId = this.keyRing.getNodeId();