fix: improve filtering of invalid nodes and add OnionPathEmptyError for better error handling

if the onion path is empty we want to refresh our snodepool cache from seed

Author: yougotwill

ts/session/apis/seed_node_api/SeedNodeAPI.ts

@@ -294,9 +294,9 @@ async function getSnodesFromSeedUrl(urlObj: URL): Promise<Array<any>> {
       );
       throw new Error(`getSnodesFromSeedUrl: json.result is empty from ${urlObj.href}`);
     }
-    // Filter 0.0.0.0 nodes which haven't submitted uptime proofs
+    // NOTE Filter out nodes that have missing ip addresses since they are not valid or 0.0.0.0 nodes which haven't submitted uptime proofs
     const validNodes = result.service_node_states.filter(
-      (snode: any) => snode.public_ip !== '0.0.0.0'
+      (snode: any) => snode.public_ip && snode.public_ip !== '0.0.0.0'
     );
 
     if (validNodes.length === 0) {

ts/session/apis/snode_api/getServiceNodesList.ts

@@ -39,9 +39,9 @@ async function getSnodePoolFromSnode(targetNode: Snode): Promise<Array<Snode>> {
       return [];
     }
 
-    // Filter 0.0.0.0 nodes which haven't submitted uptime proofs
+    // NOTE Filter out nodes that have missing ip addresses since they are not valid or 0.0.0.0 nodes which haven't submitted uptime proofs
     const snodes: Array<Snode> = json.result.service_node_states
-      .filter((snode: any) => snode.public_ip !== '0.0.0.0')
+      .filter((snode: any) => snode.public_ip && snode.public_ip !== '0.0.0.0')
       .map((snode: any) => ({
         ip: snode.public_ip,
         port: snode.storage_port,
@@ -95,7 +95,7 @@ async function getSnodePoolFromSnodes() {
     })
   );
 
-  // we want those at least `requiredSnodesForAgreement` snodes common between all the result
+  // we want those at least `requiredSnodesForAgreement` snodes common between all the results
   const commonSnodes = intersectionWith(
     results[0],
     results[1],

ts/session/apis/snode_api/getSwarmFor.ts

@@ -55,6 +55,7 @@ async function requestSnodesForPubkeyWithTargetNodeRetryable(
       throw new Error('requestSnodesForPubkey: Invalid json (empty)');
     }
 
+    // NOTE Filter out 0.0.0.0 nodes which haven't submitted uptime proofs
     const snodes = body.snodes.filter((tSnode: any) => tSnode.ip !== '0.0.0.0');
     GetNetworkTime.handleTimestampOffsetFromNetwork('get_swarm', body.t);
     return snodes;

ts/session/onions/onionPath.ts

@@ -20,6 +20,7 @@ import { ed25519Str } from '../utils/String';
 import { SnodePool } from '../apis/snode_api/snodePool';
 import { SnodePoolConstants } from '../apis/snode_api/snodePoolConstants';
 import { desiredGuardCount, minimumGuardCount, ONION_REQUEST_HOPS } from './onionPathConstants';
+import { OnionPathEmptyError } from '../utils/errors';
 
 export function getOnionPathMinTimeout() {
   return DURATION.SECONDS;
@@ -158,11 +159,14 @@ export async function getOnionPath({ toExclude }: { toExclude?: Snode }): Promis
       window.inboxStore?.dispatch(updateOnionPaths([]));
     }
   } else {
-    const ipsOnly = onionPaths.map(m =>
-      m.map(c => {
-        return { ip: c.ip };
-      })
-    );
+    const ipsOnly = onionPaths
+        // NOTE Filter out nodes that have missing ip addresses since they are not valid
+      .filter(m => m.filter(c => c.ip))
+      .map(m =>
+        m.map(c => {
+          return { ip: c.ip };
+        })
+      );
     if (!_.isEqual(window.inboxStore?.getState().onionPaths.snodePaths, ipsOnly)) {
       window.inboxStore?.dispatch(updateOnionPaths(ipsOnly));
     }
@@ -171,11 +175,11 @@ export async function getOnionPath({ toExclude }: { toExclude?: Snode }): Promis
   if (!toExclude) {
     // no need to exclude a node, then just return a random path from the list of path
     if (!onionPaths || onionPaths.length === 0) {
-      throw new Error('No onion paths available');
+      throw new OnionPathEmptyError();
     }
     const randomPathNoExclude = _.sample(onionPaths);
     if (!randomPathNoExclude) {
-      throw new Error('No onion paths available');
+      throw new OnionPathEmptyError();
     }
     return randomPathNoExclude;
   }
@@ -186,11 +190,11 @@ export async function getOnionPath({ toExclude }: { toExclude?: Snode }): Promis
   );
 
   if (!onionPathsWithoutExcluded || onionPathsWithoutExcluded.length === 0) {
-    throw new Error('No onion paths available after filtering');
+    throw new OnionPathEmptyError();
   }
   const randomPath = _.sample(onionPathsWithoutExcluded);
   if (!randomPath) {
-    throw new Error('No onion paths available after filtering');
+    throw new OnionPathEmptyError();
   }
   return randomPath;
 }

ts/session/onions/onionSend.ts

@@ -22,6 +22,8 @@ import {
 import { PROTOCOLS } from '../constants';
 import { OnionV4 } from './onionv4';
 import { MergedAbortSignal, WithAbortSignal, WithTimeoutMs } from '../apis/snode_api/requestWith';
+import { OnionPathEmptyError } from '../utils/errors';
+import { SnodePool } from '../apis/snode_api/snodePool';
 
 export type OnionFetchOptions = {
   method: string;
@@ -146,7 +148,7 @@ const sendViaOnionV4ToNonSnodeWithRetries = async (
           );
         }
         if (!pathNodes) {
-          throw new Error('getOnionPathForSending is empty');
+          throw new OnionPathEmptyError();
         }
 
         /**
@@ -243,6 +245,11 @@ const sendViaOnionV4ToNonSnodeWithRetries = async (
     );
   } catch (e) {
     window?.log?.warn('sendViaOnionV4ToNonSnodeWithRetries failed ', e.message, throwErrors);
+    // NOTE if there are no snodes available, we want to refresh the snode pool from the seed
+    if (e instanceof OnionPathEmptyError) {
+      window?.log?.warn('endViaOnionV4ToNonSnodeWithRetries failed, no path available, refreshing snode pool');
+      void SnodePool.forceRefreshRandomSnodePool();
+    }
     if (throwErrors) {
       throw e;
     }

ts/session/utils/errors.ts

@@ -81,6 +81,8 @@ class BaseError extends Error {
   }
 }
 
+// NOTE If you make a custom error with a custom message, make sure to restore the prototype chain again using the new class prototype.
+// OTHERWISE instanceof CHECKS WILL BE FALSE
 export class SigningFailed extends BaseError {}
 export class InvalidSigningType extends BaseError {}
 export class GroupV2SigningFailed extends SigningFailed {}
@@ -99,10 +101,15 @@ export class RetrieveDisplayNameError extends BaseError {
     Object.setPrototypeOf(this, RetrieveDisplayNameError.prototype);
   }
 }
-
 export class EmptyDisplayNameError extends BaseError {
   constructor(message = 'display name is empty') {
     super(message);
     Object.setPrototypeOf(this, EmptyDisplayNameError.prototype);
   }
 }
+export class OnionPathEmptyError extends BaseError {
+  constructor(message = 'onion path is empty') {
+    super(message);
+    Object.setPrototypeOf(this, OnionPathEmptyError.prototype);
+  }
+}