Merge pull request #357 from MatrixAI/feature-eng-432-review-and-refa…

…ctor-rpc-handlers-to-handle-cancellation

Updating types to match Polykey#846

npmDepsHash

@@ -1 +1 @@
-sha256-lBwUiaE6pz8zn71siGtYVRIc0rKrNQn2nzE8Z2aLN+U=
+sha256-+smJJF/RKH+OHQmb2BPqXlbHxmf5DNCOhl3Q7xQqT/U=

package.json

@@ -153,7 +153,7 @@
     "nexpect": "^0.6.0",
     "node-gyp-build": "^4.4.0",
     "nodemon": "^3.0.1",
-    "polykey": "^1.17.3",
+    "polykey": "^1.17.4",
     "prettier": "^3.0.0",
     "shelljs": "^0.8.5",
     "shx": "^0.3.4",

src/secrets/CommandCat.ts

@@ -27,6 +27,7 @@ class CommandGet extends CommandPolykey {
       const { default: PolykeyClient } = await import(
         'polykey/dist/PolykeyClient'
       );
+      const { never } = await import('polykey/dist/utils');
       const clientOptions = await binProcessors.processClientOptions(
         options.nodePath,
         options.nodeId,
@@ -95,27 +96,35 @@ class CommandGet extends CommandPolykey {
           // Print out incoming data to standard out
           let hasErrored = false;
           for await (const result of response.readable) {
-            if (result.type === 'error') {
-              hasErrored = true;
-              switch (result.code) {
-                case 'ENOENT':
-                  // Attempt to cat a non-existent file
-                  process.stderr.write(
-                    `cat: ${result.reason}: No such file or directory\n`,
-                  );
-                  break;
-                case 'EISDIR':
-                  // Attempt to cat a directory
-                  process.stderr.write(
-                    `cat: ${result.reason}: Is a directory\n`,
-                  );
-                  break;
-                default:
-                  // No other code should be thrown
-                  throw result;
-              }
-            } else {
-              process.stdout.write(result.secretContent);
+            const type = result.type;
+            switch (type) {
+              case 'ErrorMessage':
+                hasErrored = true;
+                switch (result.code) {
+                  case 'ENOENT':
+                    // Attempt to cat a non-existent file
+                    process.stderr.write(
+                      `cat: ${result.reason}: No such file or directory\n`,
+                    );
+                    break;
+                  case 'EISDIR':
+                    // Attempt to cat a directory
+                    process.stderr.write(
+                      `cat: ${result.reason}: Is a directory\n`,
+                    );
+                    break;
+                  default:
+                    // No other code should be thrown
+                    throw result;
+                }
+                break;
+              case 'SuccessMessage':
+                process.stdout.write(result.secretContent);
+                break;
+              default:
+                never(
+                  `Expected "SuccessMessage" or "ContentMessage", got ${type}`,
+                );
             }
           }
           return hasErrored;

src/secrets/CommandEdit.ts

@@ -27,10 +27,10 @@ class CommandEdit extends CommandPolykey {
       const secretPath = fullSecretPath[1] ?? '/';
       const os = await import('os');
       const { spawn } = await import('child_process');
-      const vaultsErrors = await import('polykey/dist/vaults/errors');
       const { default: PolykeyClient } = await import(
         'polykey/dist/PolykeyClient'
       );
+      const { never } = await import('polykey/dist/utils');
       const clientOptions = await binProcessors.processClientOptions(
         options.nodePath,
         options.nodeId,
@@ -65,40 +65,58 @@ class CommandEdit extends CommandPolykey {
         const tmpFile = path.join(tmpDir, path.basename(secretPath));
         const secretExists = await binUtils.retryAuthentication(
           async (auth) => {
-            let exists = true;
-            const response = await pkClient.rpcClient.methods.vaultsSecretsGet({
+            let exists = false;
+            const response =
+              await pkClient.rpcClient.methods.vaultsSecretsCat();
+            const writer = response.writable.getWriter();
+            await writer.write({
               nameOrId: vaultName,
               secretName: secretPath,
               metadata: auth,
             });
+            await writer.close();
+            const fd = await fs.promises.open(tmpFile, 'a');
             try {
-              let rawSecretContent: string = '';
-              for await (const chunk of response) {
-                rawSecretContent += chunk.secretContent;
-              }
-              const secretContent = Buffer.from(rawSecretContent, 'binary');
-              await this.fs.promises.writeFile(tmpFile, secretContent);
-            } catch (e) {
-              const [cause, _] = binUtils.remoteErrorCause(e);
-              if (cause instanceof vaultsErrors.ErrorSecretsSecretUndefined) {
-                exists = false;
-              } else if (
-                cause instanceof vaultsErrors.ErrorSecretsIsDirectory
-              ) {
-                // First, write the inline error to standard error like other
-                // secrets commands do.
-                process.stderr.write(
-                  `edit: ${secretPath}: No such file or directory\n`,
-                );
-                // Then, throw an error to get the non-zero exit code. As this
-                // command is Polykey-specific, the code doesn't really matter
-                // that much.
-                throw new errors.ErrorPolykeyCLIEditSecret(
-                  'Failed to edit secret',
-                );
-              } else {
-                throw e;
+              for await (const chunk of response.readable) {
+                const type = chunk.type;
+                switch (type) {
+                  case 'SuccessMessage':
+                    exists = true;
+                    await fd.write(Buffer.from(chunk.secretContent, 'binary'));
+                    break;
+                  case 'ErrorMessage':
+                    switch (chunk.code) {
+                      case 'ENOENT':
+                        // Do nothing if we get ENOENT. We need a case to avoid
+                        // this value hitting the default case.
+                        break;
+                      case 'EISDIR':
+                        // First, write the inline error to standard error like
+                        // other secrets commands do.
+                        process.stderr.write(
+                          `edit: ${secretPath}: No such file or directory\n`,
+                        );
+                        // Then, throw an error to get the non-zero exit code.
+                        // As this command is Polykey-specific, the code doesn't
+                        // really matter that much.
+                        throw new errors.ErrorPolykeyCLIEditSecret(
+                          'The specified secret cannot be edited',
+                        );
+                      default:
+                        throw new errors.ErrorPolykeyCLIEditSecret(
+                          `Unexpected error value returned: ${chunk.code}`,
+                        );
+                    }
+                    break;
+                  default:
+                    never(
+                      `Expected "SuccessMessage" or "ContentMessage", got ${type}`,
+                    );
+                }
               }
+            } finally {
+              await fd.close();
+              if (!exists) await fs.promises.rm(tmpFile);
             }
             return exists;
           },

src/secrets/CommandMkdir.ts

@@ -72,13 +72,10 @@ class CommandMkdir extends CommandPolykey {
             first = false;
           }
           await writer.close();
-          // Print out incoming data to standard out, or incoming errors to
-          // standard error.
+          // Print out incoming errors to standard error.
           let hasErrored = false;
-          // TypeScript cannot properly perform type narrowing on this type, so
-          // the `as` keyword is used to help it out.
           for await (const result of response.readable) {
-            if (result.type === 'error') {
+            if (result.type === 'ErrorMessage') {
               hasErrored = true;
               switch (result.code) {
                 case 'ENOENT':
@@ -98,6 +95,8 @@ class CommandMkdir extends CommandPolykey {
                   throw result;
               }
             }
+            // No additional processing needs to be done if directory creation
+            // was successful.
           }
           return hasErrored;
         }, meta);

src/secrets/CommandRemove.ts

@@ -79,7 +79,7 @@ class CommandRemove extends CommandPolykey {
           // Check if any errors were raised
           let hasErrored = false;
           for await (const result of response.readable) {
-            if (result.type === 'error') {
+            if (result.type === 'ErrorMessage') {
               hasErrored = true;
               switch (result.code) {
                 case 'ENOTEMPTY':
@@ -105,6 +105,8 @@ class CommandRemove extends CommandPolykey {
                   throw result;
               }
             }
+            // No additional processing needs to be done if file removal was
+            // successful.
           }
           return hasErrored;
         }, meta);

tests/utils/utils.ts

@@ -88,43 +88,10 @@ async function nodesConnect(localNode: PolykeyAgent, remoteNode: PolykeyAgent) {
   );
 }
 
-// This regex defines a vault secret path that always includes the secret path
-const secretPathRegex = /^([\w-]+)(?::)([^\0\\=]+)$/;
-const secretPathWithoutEnvArb = fc.stringMatching(secretPathRegex).noShrink();
-const environmentVariableAre = fc
-  .stringMatching(binParsers.environmentVariableRegex)
-  .filter((v) => v.length > 0)
-  .noShrink();
-const secretPathWithEnvArb = fc
-  .tuple(secretPathWithoutEnvArb, environmentVariableAre)
-  .map((v) => v.join('='));
-const secretPathEnvArb = fc.oneof(
-  secretPathWithoutEnvArb,
-  secretPathWithEnvArb,
-);
-
-const secretPathEnvArrayArb = fc
-  .array(secretPathEnvArb, { minLength: 1, size: 'small' })
-  .noShrink();
-const cmdArgsArrayArb = fc
-  .array(fc.oneof(fc.string(), secretPathEnvArb, fc.constant('--')), {
-    size: 'small',
-  })
-  .noShrink();
-
 const vaultNameArb = fc
   .string({ minLength: 1, maxLength: 100 })
   .filter((str) => binParsers.vaultNameRegex.test(str))
   .filter((str) => !str.startsWith('-'))
   .noShrink();
 
-export {
-  testIf,
-  describeIf,
-  trackTimers,
-  nodesConnect,
-  secretPathEnvArb,
-  secretPathEnvArrayArb,
-  cmdArgsArrayArb,
-  vaultNameArb,
-};
+export { testIf, describeIf, trackTimers, nodesConnect, vaultNameArb };