Skip to content

Commit 51dfa4a

Browse files
aryanjassalaryanjassal
authored
Merge pull request #357 from MatrixAI/feature-eng-432-review-and-refactor-rpc-handlers-to-handle-cancellation
Updating types to match Polykey#846
2 parents 6a390a0 + 720eff9 commit 51dfa4a

File tree

8 files changed

+91
-96
lines changed

8 files changed

+91
-96
lines changed

‎npmDepsHash‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
sha256-lBwUiaE6pz8zn71siGtYVRIc0rKrNQn2nzE8Z2aLN+U=
1+
sha256-+smJJF/RKH+OHQmb2BPqXlbHxmf5DNCOhl3Q7xQqT/U=

‎package-lock.json‎

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@
153153
"nexpect": "^0.6.0",
154154
"node-gyp-build": "^4.4.0",
155155
"nodemon": "^3.0.1",
156-
"polykey": "^1.17.3",
156+
"polykey": "^1.17.4",
157157
"prettier": "^3.0.0",
158158
"shelljs": "^0.8.5",
159159
"shx": "^0.3.4",

‎src/secrets/CommandCat.ts‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ class CommandGet extends CommandPolykey {
2727
const { default: PolykeyClient } = await import(
2828
'polykey/dist/PolykeyClient'
2929
);
30+
const { never } = await import('polykey/dist/utils');
3031
const clientOptions = await binProcessors.processClientOptions(
3132
options.nodePath,
3233
options.nodeId,
@@ -95,27 +96,35 @@ class CommandGet extends CommandPolykey {
9596
// Print out incoming data to standard out
9697
let hasErrored = false;
9798
for await (const result of response.readable) {
98-
if (result.type === 'error') {
99-
hasErrored = true;
100-
switch (result.code) {
101-
case 'ENOENT':
102-
// Attempt to cat a non-existent file
103-
process.stderr.write(
104-
`cat: ${result.reason}: No such file or directory\n`,
105-
);
106-
break;
107-
case 'EISDIR':
108-
// Attempt to cat a directory
109-
process.stderr.write(
110-
`cat: ${result.reason}: Is a directory\n`,
111-
);
112-
break;
113-
default:
114-
// No other code should be thrown
115-
throw result;
116-
}
117-
} else {
118-
process.stdout.write(result.secretContent);
99+
const type = result.type;
100+
switch (type) {
101+
case 'ErrorMessage':
102+
hasErrored = true;
103+
switch (result.code) {
104+
case 'ENOENT':
105+
// Attempt to cat a non-existent file
106+
process.stderr.write(
107+
`cat: ${result.reason}: No such file or directory\n`,
108+
);
109+
break;
110+
case 'EISDIR':
111+
// Attempt to cat a directory
112+
process.stderr.write(
113+
`cat: ${result.reason}: Is a directory\n`,
114+
);
115+
break;
116+
default:
117+
// No other code should be thrown
118+
throw result;
119+
}
120+
break;
121+
case 'SuccessMessage':
122+
process.stdout.write(result.secretContent);
123+
break;
124+
default:
125+
never(
126+
`Expected "SuccessMessage" or "ContentMessage", got ${type}`,
127+
);
119128
}
120129
}
121130
return hasErrored;

‎src/secrets/CommandEdit.ts‎

Lines changed: 47 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@ class CommandEdit extends CommandPolykey {
2727
const secretPath = fullSecretPath[1] ?? '/';
2828
const os = await import('os');
2929
const { spawn } = await import('child_process');
30-
const vaultsErrors = await import('polykey/dist/vaults/errors');
3130
const { default: PolykeyClient } = await import(
3231
'polykey/dist/PolykeyClient'
3332
);
33+
const { never } = await import('polykey/dist/utils');
3434
const clientOptions = await binProcessors.processClientOptions(
3535
options.nodePath,
3636
options.nodeId,
@@ -65,40 +65,58 @@ class CommandEdit extends CommandPolykey {
6565
const tmpFile = path.join(tmpDir, path.basename(secretPath));
6666
const secretExists = await binUtils.retryAuthentication(
6767
async (auth) => {
68-
let exists = true;
69-
const response = await pkClient.rpcClient.methods.vaultsSecretsGet({
68+
let exists = false;
69+
const response =
70+
await pkClient.rpcClient.methods.vaultsSecretsCat();
71+
const writer = response.writable.getWriter();
72+
await writer.write({
7073
nameOrId: vaultName,
7174
secretName: secretPath,
7275
metadata: auth,
7376
});
77+
await writer.close();
78+
const fd = await fs.promises.open(tmpFile, 'a');
7479
try {
75-
let rawSecretContent: string = '';
76-
for await (const chunk of response) {
77-
rawSecretContent += chunk.secretContent;
78-
}
79-
const secretContent = Buffer.from(rawSecretContent, 'binary');
80-
await this.fs.promises.writeFile(tmpFile, secretContent);
81-
} catch (e) {
82-
const [cause, _] = binUtils.remoteErrorCause(e);
83-
if (cause instanceof vaultsErrors.ErrorSecretsSecretUndefined) {
84-
exists = false;
85-
} else if (
86-
cause instanceof vaultsErrors.ErrorSecretsIsDirectory
87-
) {
88-
// First, write the inline error to standard error like other
89-
// secrets commands do.
90-
process.stderr.write(
91-
`edit: ${secretPath}: No such file or directory\n`,
92-
);
93-
// Then, throw an error to get the non-zero exit code. As this
94-
// command is Polykey-specific, the code doesn't really matter
95-
// that much.
96-
throw new errors.ErrorPolykeyCLIEditSecret(
97-
'Failed to edit secret',
98-
);
99-
} else {
100-
throw e;
80+
for await (const chunk of response.readable) {
81+
const type = chunk.type;
82+
switch (type) {
83+
case 'SuccessMessage':
84+
exists = true;
85+
await fd.write(Buffer.from(chunk.secretContent, 'binary'));
86+
break;
87+
case 'ErrorMessage':
88+
switch (chunk.code) {
89+
case 'ENOENT':
90+
// Do nothing if we get ENOENT. We need a case to avoid
91+
// this value hitting the default case.
92+
break;
93+
case 'EISDIR':
94+
// First, write the inline error to standard error like
95+
// other secrets commands do.
96+
process.stderr.write(
97+
`edit: ${secretPath}: No such file or directory\n`,
98+
);
99+
// Then, throw an error to get the non-zero exit code.
100+
// As this command is Polykey-specific, the code doesn't
101+
// really matter that much.
102+
throw new errors.ErrorPolykeyCLIEditSecret(
103+
'The specified secret cannot be edited',
104+
);
105+
default:
106+
throw new errors.ErrorPolykeyCLIEditSecret(
107+
`Unexpected error value returned: ${chunk.code}`,
108+
);
109+
}
110+
break;
111+
default:
112+
never(
113+
`Expected "SuccessMessage" or "ContentMessage", got ${type}`,
114+
);
115+
}
101116
}
117+
} finally {
118+
await fd.close();
119+
if (!exists) await fs.promises.rm(tmpFile);
102120
}
103121
return exists;
104122
},

‎src/secrets/CommandMkdir.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,13 +72,10 @@ class CommandMkdir extends CommandPolykey {
7272
first = false;
7373
}
7474
await writer.close();
75-
// Print out incoming data to standard out, or incoming errors to
76-
// standard error.
75+
// Print out incoming errors to standard error.
7776
let hasErrored = false;
78-
// TypeScript cannot properly perform type narrowing on this type, so
79-
// the `as` keyword is used to help it out.
8077
for await (const result of response.readable) {
81-
if (result.type === 'error') {
78+
if (result.type === 'ErrorMessage') {
8279
hasErrored = true;
8380
switch (result.code) {
8481
case 'ENOENT':
@@ -98,6 +95,8 @@ class CommandMkdir extends CommandPolykey {
9895
throw result;
9996
}
10097
}
98+
// No additional processing needs to be done if directory creation
99+
// was successful.
101100
}
102101
return hasErrored;
103102
}, meta);

‎src/secrets/CommandRemove.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ class CommandRemove extends CommandPolykey {
7979
// Check if any errors were raised
8080
let hasErrored = false;
8181
for await (const result of response.readable) {
82-
if (result.type === 'error') {
82+
if (result.type === 'ErrorMessage') {
8383
hasErrored = true;
8484
switch (result.code) {
8585
case 'ENOTEMPTY':
@@ -105,6 +105,8 @@ class CommandRemove extends CommandPolykey {
105105
throw result;
106106
}
107107
}
108+
// No additional processing needs to be done if file removal was
109+
// successful.
108110
}
109111
return hasErrored;
110112
}, meta);

‎tests/utils/utils.ts‎

Lines changed: 1 addition & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -88,43 +88,10 @@ async function nodesConnect(localNode: PolykeyAgent, remoteNode: PolykeyAgent) {
8888
);
8989
}
9090

91-
// This regex defines a vault secret path that always includes the secret path
92-
const secretPathRegex = /^([\w-]+)(?::)([^\0\\=]+)$/;
93-
const secretPathWithoutEnvArb = fc.stringMatching(secretPathRegex).noShrink();
94-
const environmentVariableAre = fc
95-
.stringMatching(binParsers.environmentVariableRegex)
96-
.filter((v) => v.length > 0)
97-
.noShrink();
98-
const secretPathWithEnvArb = fc
99-
.tuple(secretPathWithoutEnvArb, environmentVariableAre)
100-
.map((v) => v.join('='));
101-
const secretPathEnvArb = fc.oneof(
102-
secretPathWithoutEnvArb,
103-
secretPathWithEnvArb,
104-
);
105-
106-
const secretPathEnvArrayArb = fc
107-
.array(secretPathEnvArb, { minLength: 1, size: 'small' })
108-
.noShrink();
109-
const cmdArgsArrayArb = fc
110-
.array(fc.oneof(fc.string(), secretPathEnvArb, fc.constant('--')), {
111-
size: 'small',
112-
})
113-
.noShrink();
114-
11591
const vaultNameArb = fc
11692
.string({ minLength: 1, maxLength: 100 })
11793
.filter((str) => binParsers.vaultNameRegex.test(str))
11894
.filter((str) => !str.startsWith('-'))
11995
.noShrink();
12096

121-
export {
122-
testIf,
123-
describeIf,
124-
trackTimers,
125-
nodesConnect,
126-
secretPathEnvArb,
127-
secretPathEnvArrayArb,
128-
cmdArgsArrayArb,
129-
vaultNameArb,
130-
};
97+
export { testIf, describeIf, trackTimers, nodesConnect, vaultNameArb };

0 commit comments

Comments
 (0)