Refactor error handling of failed Node Connections created from the Discovery domain

[emmacasolin]

Specification

While completing #311 it was discovered that failed node connections created by the Discovery domain (using NodeManager.requestChainData() to get the sigchain data of a node during the discovery process) were not handled correctly, leading to strange errors during testing. A quick fix for handling these errors was implemented, however it could be refactored to be more specific.

Currently we have (inside src/discovery/Discovery.ts):

// Lines 234-246 (requesting chain data from a node in the discovery queue)
try {
  vertexChainData = await this.nodeManager.requestChainData(
    nodeId,
  );
} catch (e) {
  this.visitedVertices.add(vertex);
  await this.removeKeyFromDiscoveryQueue(vertexId);
  this.logger.error(
    `Failed to discover ${vertexGId.nodeId} - ${e.toString()}`,
  );
  yield;
  continue;
}

// Lines 281-302 (requesting chain data from a node that is linked to a node in the discovery queue)
try {
  linkedVertexChainData =
    await this.nodeManager.requestChainData(linkedVertexNodeId);
} catch (e) {
  if (
    e instanceof nodesErrors.ErrorNodeConnectionDestroyed ||
    e instanceof nodesErrors.ErrorNodeConnectionTimeout
  ) {
    if (!this.visitedVertices.has(linkedVertexGK)) {
      await this.pushKeyToDiscoveryQueue(linkedVertexGK);
    }
    this.logger.error(
      `Failed to discover ${nodesUtils.encodeNodeId(
          linkedVertexNodeId,
       )} - ${e.toString()}`,
    );
    yield;
    continue;
  } else {
    throw e;
  }
}

// Lines 367-387 (requesting chain data from a node that is linked to an identity in the discovery queue)
try {
  linkedVertexChainData = await this.nodeManager.requestChainData(
    linkedVertexNodeId,
  );
} catch (e) {
  if (
    e instanceof nodesErrors.ErrorNodeConnectionDestroyed ||
    e instanceof nodesErrors.ErrorNodeConnectionTimeout
  ) {
    if (!this.visitedVertices.has(linkedVertexGK)) {
      await this.pushKeyToDiscoveryQueue(linkedVertexGK);
    }
    yield;
    this.logger.error(
      `Failed to discover ${data.node} - ${e.toString()}`,
    );
    continue;
  } else {
    throw e;
  }
}

Additional context

• Original discussion of error propagation in this situation: CLI and Client & Agent Service test splitting #311 (comment)
• Issue for writing tests for the errors that may occur here (progress on these two issues may affect the other): Write tests to cover all possible states of the Discovery, NodeConnection, and discovered Node during the discovery process #349

Tasks

1. Ensure that in each place in the code only the relevant errors are caught and logged out (i.e. ErrorNodeConnectionDestroyed, ErrorNodeConnectionTimeout, and any other errors found while working through Write tests to cover all possible states of the Discovery, NodeConnection, and discovered Node during the discovery process #349)
2. Ensure that after an error is caught the order of subsequent actions is (1) log out the error (2) yield (3) continue