Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI Retry Options due to Intermittent Network #325

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 4 additions & 23 deletions src/bin/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,28 +51,12 @@ class ErrorCLIFileRead extends ErrorCLI {
exitCode = sysexits.NOINPUT;
}

class ErrorSecretPathFormat extends ErrorCLI {
description = "Secret name needs to be of format: '<vaultName>:<secretPath>'";
exitCode = 64;
}

class ErrorVaultNameAmbiguous extends ErrorCLI {
description =
'There is more than 1 Vault with this name. Please specify a Vault ID';
exitCode = 1;
}

class ErrorSecretsUndefined extends ErrorCLI {
description = 'At least one secret must be specified as an argument';
exitCode = 64;
}

class ErrorNodeFindFailed extends ErrorCLI {
class ErrorCLINodeFindFailed extends ErrorCLI {
description = 'Failed to find the node in the DHT';
exitCode = 1;
}

class ErrorNodePingFailed extends ErrorCLI {
class ErrorCLINodePingFailed extends ErrorCLI {
description = 'Node was not online or not found.';
exitCode = 1;
}
Expand All @@ -88,9 +72,6 @@ export {
ErrorCLIPasswordFileRead,
ErrorCLIRecoveryCodeFileRead,
ErrorCLIFileRead,
ErrorSecretPathFormat,
ErrorVaultNameAmbiguous,
ErrorSecretsUndefined,
ErrorNodeFindFailed,
ErrorNodePingFailed,
ErrorCLINodeFindFailed,
ErrorCLINodePingFailed,
};
63 changes: 40 additions & 23 deletions src/bin/nodes/CommandFind.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import * as binOptions from '../utils/options';
import * as binProcessors from '../utils/processors';
import * as binParsers from '../utils/parsers';
import * as binErrors from '../errors';
import { sleep } from '../../utils';

class CommandFind extends CommandPolykey {
constructor(...args: ConstructorParameters<typeof CommandPolykey>) {
Expand All @@ -17,6 +18,8 @@ class CommandFind extends CommandPolykey {
this.addOption(binOptions.nodeId);
this.addOption(binOptions.clientHost);
this.addOption(binOptions.clientPort);
this.addOption(binOptions.retryCount);
this.addOption(binOptions.retryInterval);
this.action(async (nodeId: NodeId, options) => {
const { default: PolykeyClient } = await import('../../PolykeyClient');
const nodesPB = await import('../../proto/js/polykey/v1/nodes/nodes_pb');
Expand Down Expand Up @@ -57,28 +60,42 @@ class CommandFind extends CommandPolykey {
host: '',
port: 0,
};
try {
const response = await binUtils.retryAuthentication(
(auth) => pkClient.grpcClient.nodesFind(nodeMessage, auth),
meta,
);
result.success = true;
result.id = response.getNodeId();
result.host = response.getAddress()!.getHost();
result.port = response.getAddress()!.getPort();
result.message = `Found node at ${networkUtils.buildAddress(
result.host as Host,
result.port as Port,
)}`;
} catch (err) {
if (!(err instanceof nodesErrors.ErrorNodeGraphNodeNotFound))
throw err;
// Else failed to find the node.
result.success = false;
result.id = nodesUtils.encodeNodeId(nodeId);
result.host = '';
result.port = 0;
result.message = `Failed to find node ${result.id}`;
let attemptCount = 1;
const maxAttempts: number | undefined = options.retryCount;
const attemptDelay: number = options.retryInterval;
while (true) {
try {
const response = await binUtils.retryAuthentication(
(auth) => pkClient.grpcClient.nodesFind(nodeMessage, auth),
meta,
);
result.success = true;
result.id = response.getNodeId();
result.host = response.getAddress()!.getHost();
result.port = response.getAddress()!.getPort();
result.message = `Found node at ${networkUtils.buildAddress(
result.host as Host,
result.port as Port,
)}`;
break;
} catch (err) {
if (!(err instanceof nodesErrors.ErrorNodeGraphNodeNotFound)) {
throw err;
}
if (maxAttempts !== undefined && attemptCount < maxAttempts) {
// Delay and continue.
attemptCount++;
await sleep(attemptDelay);
continue;
}
// Else failed to find the node.
result.success = false;
result.id = nodesUtils.encodeNodeId(nodeId);
result.host = '';
result.port = 0;
result.message = `Failed to find node ${result.id}`;
break;
}
}
let output: any = result;
if (options.format === 'human') output = [result.message];
Expand All @@ -90,7 +107,7 @@ class CommandFind extends CommandPolykey {
);
// Like ping it should error when failing to find node for automation reasons.
if (!result.success)
throw new binErrors.ErrorNodeFindFailed(result.message);
throw new binErrors.ErrorCLINodeFindFailed(result.message);
} finally {
if (pkClient! != null) await pkClient.stop();
}
Expand Down
50 changes: 35 additions & 15 deletions src/bin/nodes/CommandPing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as binOptions from '../utils/options';
import * as binProcessors from '../utils/processors';
import * as binParsers from '../utils/parsers';
import * as binErrors from '../errors';
import { sleep } from '../../utils';

class CommandPing extends CommandPolykey {
constructor(...args: ConstructorParameters<typeof CommandPolykey>) {
Expand All @@ -16,6 +17,8 @@ class CommandPing extends CommandPolykey {
this.addOption(binOptions.nodeId);
this.addOption(binOptions.clientHost);
this.addOption(binOptions.clientPort);
this.addOption(binOptions.retryCount);
this.addOption(binOptions.retryInterval);
this.action(async (nodeId: NodeId, options) => {
const { default: PolykeyClient } = await import('../../PolykeyClient');
const nodesUtils = await import('../../nodes/utils');
Expand Down Expand Up @@ -49,26 +52,43 @@ class CommandPing extends CommandPolykey {
nodeMessage.setNodeId(nodesUtils.encodeNodeId(nodeId));
let statusMessage;
let error;
try {
statusMessage = await binUtils.retryAuthentication(
(auth) => pkClient.grpcClient.nodesPing(nodeMessage, auth),
meta,
);
} catch (err) {
if (err instanceof nodesErrors.ErrorNodeGraphNodeNotFound) {
error = new binErrors.ErrorNodePingFailed(
`Failed to resolve node ID ${nodesUtils.encodeNodeId(
nodeId,
)} to an address.`,
let attemptCount = 1;
const maxAttempts: number | undefined = options.retryCount;
const attemptDelay: number = options.retryInterval;
while (true) {
try {
statusMessage = await binUtils.retryAuthentication(
(auth) => pkClient.grpcClient.nodesPing(nodeMessage, auth),
meta,
);
} else {
throw err;
if (statusMessage.getSuccess()) break;
if (maxAttempts !== undefined && attemptCount < maxAttempts) {
// Delay and continue
attemptCount++;
await sleep(attemptDelay);
continue;
}
break;
} catch (err) {
if (err instanceof nodesErrors.ErrorNodeGraphNodeNotFound)
throw err;
if (maxAttempts !== undefined && attemptCount < maxAttempts) {
// Delay and continue
attemptCount++;
await sleep(attemptDelay);
continue;
}
error = new binErrors.ErrorCLINodePingFailed(
`Failed to resolve node ID ${nodesUtils.encodeNodeId(nodeId)} to an address`,
);
break;
}
}
const status = { success: false, message: '' };
status.success = statusMessage ? statusMessage.getSuccess() : false;
if (!status.success && !error)
error = new binErrors.ErrorNodePingFailed('No response received');
if (!status.success && !error) {
error = new binErrors.ErrorCLINodePingFailed('No response received');
}
if (status.success) status.message = 'Node is Active.';
else status.message = error.message;
const output: any =
Expand Down
15 changes: 15 additions & 0 deletions src/bin/utils/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,19 @@ const workers = new commander.Option(
.argParser(binParsers.parseCoreCount)
.default(undefined);


const retryCount = new commander.Option(
'-rc --retry-count <retryCount>',
'Number of attempts before failing',
).argParser(binParsers.parseNumber);

const retryInterval = new commander.Option(
'-ri --retry-interval <retryInterval>',
'Number of milliseconds between each attempt',
)
.argParser(binParsers.parseNumber)
.default(5000);

export {
nodePath,
format,
Expand All @@ -176,4 +189,6 @@ export {
seedNodes,
network,
workers,
retryCount,
retryInterval,
};
4 changes: 4 additions & 0 deletions tests/bin/nodes/find.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ describe('find', () => {
const commands = genCommands([
'find',
nodesUtils.encodeNodeId(unknownNodeId),
'-c',
'3',
'-ad',
'100',
]);
const result = await testBinUtils.pkStdio(commands, {}, dataDir);
expect(result.exitCode).toBe(1);
Expand Down
4 changes: 4 additions & 0 deletions tests/bin/nodes/ping.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ describe('ping', () => {
const commands = genCommands([
'ping',
nodesUtils.encodeNodeId(remoteOfflineNodeId),
'-c',
'1',
'-ad',
'100',
]);
const result = await testBinUtils.pkStdio(commands, {}, dataDir);
expect(result.exitCode).toBe(1); // Should fail with no response. for automation purposes.
Expand Down