Merge pull request #320 from MatrixAI/feature-undefined-errors

Properly display non-Polykey errors instead of printing `undefined`

Author: aryanjassal

npmDepsHash

@@ -1 +1 @@
-sha256-/JZlQA4VOEGvT5iFaN2KN4LpKI0RqlFhmFQJlesKRQU=
+sha256-WkSKOsHFtWeCLyYr5WICTXG+A4+P6/hdxvlutNudQTg=

package-lock.json

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

package.json

@@ -75,6 +75,15 @@ class ErrorPolykeyCLIAsynchronousDeadlock<T> extends ErrorPolykeyCLI<T> {
   exitCode = sysexits.SOFTWARE;
 }
 
+/**
+ * Unexpected error is a runtime error.
+ * If these exceptions occur, there is a bug. Most likely in Polykey.
+ */
+class ErrorPolykeyCLIUnexpectedError<T> extends ErrorPolykeyCLI<T> {
+  static description = 'An unexpected error occured';
+  exitCode = sysexits.SOFTWARE;
+}
+
 class ErrorPolykeyCLINodePath<T> extends ErrorPolykeyCLI<T> {
   static description = 'Cannot derive default node path from unknown platform';
   exitCode = sysexits.USAGE;
@@ -172,10 +181,16 @@ class ErrorPolykeyCLICatSecret<T> extends ErrorPolykeyCLI<T> {
   exitCode = 1;
 }
 
+class ErrorPolykeyCLIEditSecret<T> extends ErrorPolykeyCLI<T> {
+  static description = 'Failed to edit a secret';
+  exitCode = 1;
+}
+
 export {
   ErrorPolykeyCLI,
   ErrorPolykeyCLIUncaughtException,
   ErrorPolykeyCLIUnhandledRejection,
+  ErrorPolykeyCLIUnexpectedError,
   ErrorPolykeyCLIAsynchronousDeadlock,
   ErrorPolykeyCLINodePath,
   ErrorPolykeyCLIClientOptions,
@@ -196,4 +211,5 @@ export {
   ErrorPolykeyCLIRenameSecret,
   ErrorPolykeyCLIRemoveSecret,
   ErrorPolykeyCLICatSecret,
+  ErrorPolykeyCLIEditSecret,
 };

src/errors.ts

@@ -75,10 +75,14 @@ async function polykeyAgentMain(): Promise<number> {
       process.exitCode = e.exitCode;
     } else {
       // Unknown error, this should not happen
+      const wrappedError = new binErrors.ErrorPolykeyCLIUnexpectedError(
+        binUtils.composeErrorMessage(e),
+        { cause: e },
+      );
       process.stderr.write(
         binUtils.outputFormatter({
           type: errFormat,
-          data: e,
+          data: wrappedError,
         }),
       );
       process.exitCode = 255;
@@ -217,10 +221,14 @@ async function polykeyMain(argv: Array<string>): Promise<number> {
       process.exitCode = e.exitCode;
     } else {
       // Unknown error, this should not happen
+      const wrappedError = new binErrors.ErrorPolykeyCLIUnexpectedError(
+        binUtils.composeErrorMessage(e),
+        { cause: e },
+      );
       process.stderr.write(
         binUtils.outputFormatter({
           type: errFormat,
-          data: e,
+          data: wrappedError,
         }),
       );
       process.exitCode = 255;

src/polykey.ts

@@ -1,4 +1,6 @@
 import type PolykeyClient from 'polykey/dist/PolykeyClient';
+import type { ContentOrErrorMessage } from 'polykey/dist/client/types';
+import type { ReadableStream } from 'stream/web';
 import CommandPolykey from '../CommandPolykey';
 import * as binUtils from '../utils';
 import * as binOptions from '../utils/options';
@@ -80,28 +82,44 @@ class CommandGet extends CommandPolykey {
         }
         const hasErrored = await binUtils.retryAuthentication(async (auth) => {
           // Write secret paths to input stream
-          const response = await pkClient.rpcClient.methods.vaultsSecretsGet();
+          const response = await pkClient.rpcClient.methods.vaultsSecretsCat();
           const writer = response.writable.getWriter();
           let first = true;
           for (const [vaultName, secretPath] of secretPaths) {
             await writer.write({
               nameOrId: vaultName,
               secretName: secretPath ?? '/',
-              metadata: first
-                ? { ...auth, options: { continueOnError: true } }
-                : undefined,
+              metadata: first ? auth : undefined,
             });
             first = false;
           }
           await writer.close();
           // Print out incoming data to standard out
           let hasErrored = false;
-          for await (const chunk of response.readable) {
-            if (chunk.error) {
+          // 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 as ReadableStream<ContentOrErrorMessage>) {
+            if (result.type === 'error') {
               hasErrored = true;
-              process.stderr.write(chunk.error);
+              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(chunk.secretContent);
+              process.stdout.write(result.secretContent);
             }
           }
           return hasErrored;

src/secrets/CommandCat.ts

@@ -24,7 +24,7 @@ class CommandEdit extends CommandPolykey {
     this.addOption(binOptions.clientPort);
     this.action(async (secretPath, options) => {
       const os = await import('os');
-      const { execSync } = await import('child_process');
+      const { spawn } = await import('child_process');
       const vaultsErrors = await import('polykey/dist/vaults/errors');
       const { default: PolykeyClient } = await import(
         'polykey/dist/PolykeyClient'
@@ -64,17 +64,14 @@ class CommandEdit extends CommandPolykey {
         const secretExists = await binUtils.retryAuthentication(
           async (auth) => {
             let exists = true;
-            const res = await pkClient.rpcClient.methods.vaultsSecretsGet();
-            const writer = res.writable.getWriter();
-            await writer.write({
+            const response = await pkClient.rpcClient.methods.vaultsSecretsGet({
               nameOrId: secretPath[0],
               secretName: secretPath[1] ?? '/',
               metadata: auth,
             });
-            await writer.close();
             try {
               let rawSecretContent: string = '';
-              for await (const chunk of res.readable) {
+              for await (const chunk of response) {
                 rawSecretContent += chunk.secretContent;
               }
               const secretContent = Buffer.from(rawSecretContent, 'binary');
@@ -83,6 +80,20 @@ class CommandEdit extends CommandPolykey {
               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[1] ?? '/'}: 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;
               }
@@ -94,7 +105,29 @@ class CommandEdit extends CommandPolykey {
         // If the editor exited with a code other than zero, then execSync
         // will throw an error. So, in the case of saving the file but the
         // editor crashing, the program won't save the updated secret.
-        execSync(`${process.env.EDITOR} \"${tmpFile}\"`, { stdio: 'inherit' });
+        await new Promise<void>((resolve, reject) => {
+          // If $EDITOR is unset, default to nano. If that doesn't exist, then
+          // the command will raise an error.
+          const editorProc = spawn(process.env.EDITOR ?? 'nano', [tmpFile], {
+            stdio: 'inherit',
+          });
+          editorProc.on('error', (e) => {
+            const error = new errors.ErrorPolykeyCLIEditSecret(
+              `Failed to run command ${process.env.EDITOR}`,
+              { cause: e },
+            );
+            reject(error);
+          });
+          editorProc.on('close', (code) => {
+            if (code !== 0) {
+              const error = new errors.ErrorPolykeyCLIEditSecret(
+                `Editor exited with code ${code}`,
+              );
+              reject(error);
+            }
+            resolve();
+          });
+        });
         let content: string;
         try {
           const buffer = await this.fs.promises.readFile(tmpFile);

src/secrets/CommandEdit.ts

@@ -1,14 +1,12 @@
 import type PolykeyClient from 'polykey/dist/PolykeyClient';
-import type { ErrorMessage } from 'polykey/dist/client/types';
+import type { SuccessOrErrorMessage } from 'polykey/dist/client/types';
+import type { ReadableStream } from 'stream/web';
 import CommandPolykey from '../CommandPolykey';
 import * as binUtils from '../utils';
 import * as binOptions from '../utils/options';
 import * as binParsers from '../utils/parsers';
 import * as binProcessors from '../utils/processors';
-import {
-  ErrorPolykeyCLIMakeDirectory,
-  ErrorPolykeyCLIUncaughtException,
-} from '../errors';
+import { ErrorPolykeyCLIMakeDirectory } from '../errors';
 
 class CommandMkdir extends CommandPolykey {
   constructor(...args: ConstructorParameters<typeof CommandPolykey>) {
@@ -79,29 +77,28 @@ class CommandMkdir extends CommandPolykey {
           // Print out incoming data to standard out, or incoming errors to
           // standard error.
           let hasErrored = false;
-          for await (const result of response.readable) {
+          // 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 as ReadableStream<SuccessOrErrorMessage>) {
             if (result.type === 'error') {
-              // TS cannot properly evaluate a type this deeply nested, so we use
-              // the as keyword to help it. Inside this block, the type of data
-              // is ensured to be 'error'.
-              const error = result as ErrorMessage;
               hasErrored = true;
-              let message: string = '';
-              switch (error.code) {
+              switch (result.code) {
                 case 'ENOENT':
-                  message = 'No such secret or directory';
+                  // Attempt to create a directory without existing parents
+                  process.stderr.write(
+                    `mkdir: cannot create directory ${result.reason}: No such file or directory\n`,
+                  );
                   break;
                 case 'EEXIST':
-                  message = 'Secret or directory exists';
+                  // Attempt to create a directory where something already exists
+                  process.stderr.write(
+                    `mkdir: cannot create directory ${result.reason}: File or directory exists\n`,
+                  );
                   break;
                 default:
-                  throw new ErrorPolykeyCLIUncaughtException(
-                    `Unexpected error code: ${error.code}`,
-                  );
+                  // No other code should be thrown
+                  throw result;
               }
-              process.stderr.write(
-                `${error.code}: cannot create directory ${error.reason}: ${message}\n`,
-              );
             }
           }
           return hasErrored;