Polykey
Polykey copied to clipboard
Refactor error handling of failed Node Connections created from the Discovery domain
Specification
While completing https://github.com/MatrixAI/js-polykey/pull/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: https://github.com/MatrixAI/js-polykey/pull/311#issuecomment-1049484685
- Issue for writing tests for the errors that may occur here (progress on these two issues may affect the other): https://github.com/MatrixAI/js-polykey/issues/349
Tasks
- 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 #349) - Ensure that after an error is caught the order of subsequent actions is (1) log out the error (2) yield (3) continue