Merge pull request #405 from hashicorp/feature-ipc-errors

Feature ipc errors

ui/desktop/app/routes/scopes/scope/projects/targets.js

@@ -49,10 +49,10 @@ export default class ScopesScopeProjectsTargetsRoute extends Route {
     try {
       await this.ipc.invoke('connect', {
         target_id: model.target.id,
-        auth_token: this.session.data.authenticated.token,
+        token: this.session.data.authenticated.token,
       });
     } catch(e) {
-      this.notify.error(e.toString(), { closeAfter: null });
+      this.notify.error(e.message, { closeAfter: null });
     }
   }
 }

ui/desktop/app/services/ipc.js

@@ -59,10 +59,14 @@ export class IPCRequest {
     this.payload = payload;
     this.origin = window?.location?.origin;
     this.#channel = new MessageChannel();
-    this.#promise = new Promise((resolve /*, reject*/) => {
+    this.#promise = new Promise((resolve, reject) => {
       this.#channel.port1.onmessage = (event) => {
         this.close();
-        resolve(event.data);
+        if (event.data instanceof Error) {
+          reject(JSON.parse(event.data.message));
+        } else {
+          resolve(event.data);
+        }
       };
     });
     window.postMessage(

ui/desktop/electron-app/src/handlers.js

@@ -1,21 +1,14 @@
-const { ipcMain } = require('electron');
-const isDev = require('electron-is-dev');
-const log = (method, request) => {
-  if (isDev) console.log(`[ipc-main](${method}): `, request);
-};
+const handle = require('./ipc-handler');
 const { lookpath } = require('lookpath');
 const { exec } = require('child_process');
 
-let origin = 'serve://boundary';
+let origin = '';
 
 /**
  * Returns the current runtime origin, which is used by the main thread to
  * rewrite the CSP to allow requests.
  */
-ipcMain.handle('getOrigin', async (event, request) => {
-  log('getOrigin', request);
-  return origin;
-});
+handle('getOrigin', () => origin);
 
 /**
  * TODO:  sets the main process origin to the specified value.  This value will
@@ -25,29 +18,53 @@ ipcMain.handle('getOrigin', async (event, request) => {
  *
  * For now, we update the internal origin but do not yet rewrite the CSP.
  */
-ipcMain.handle('setOrigin', async (event, request) => {
-  log('setOrigin', request);
-  origin = request?.data;
-  return origin;
-});
+handle('setOrigin', (requestOrigin) => origin = requestOrigin);
 
 /**
  * Establishes a boundary session and returns session details.
  * TODO: Return session details.
  */
-ipcMain.handle('connect', async (event, request) => {
-  log('connect payload', request);
-
+handle('connect', async ({ target_id, token }) => {
   const cliAvailable = await lookpath('boundary');
-  log('cli check', cliAvailable);
   if(!cliAvailable) { throw new Error('CLI unavailable.'); }
 
-  const connectCmd = `boundary connect -target-id=${request.target_id} -token=${request.auth_token}`;
-  log('connect command', connectCmd);
+  const connectCmd = `boundary connect -target-id=${target_id} -token=${token}`;
 
-  exec(connectCmd, (output) => {
-    log('connect output', output);
-  }, (error) => {
-    log('connect error', error);
+  // TODO:  this isn't the *exact* promise, because apparently exec doesn't
+  // call the success callback until the process exits.  We need a way to
+  // resolve the promise if the process doesn't exit AND we receive valid
+  // JSON on stdout, since this indicates the connection was successful.
+  //
+  // TODO:  if there is an existing promiserified exec tool, we might consider
+  // using it to help make this cleaner.
+
+  // You can throw exceptions, or allow them to occur, and this is supported.
+  // Exceptions thrown in this way will be returned to the UI as an
+  // IPC rejection. However in most cases, you probably want to return a
+  // specific exception object of your own design, since Error instances
+  // originating from the underlying system will be noisey and not very helpful
+  // to users.
+  //
+  //throw new Error('Random error!  Should still work!');
+  //throw { message: 'Random POJO exception!  Should still work!' };
+
+  return new Promise((resolve, reject) => {
+    exec(connectCmd, (error, stdout, stderr) => {
+      if (error) {
+        // For example, you could just reject(error) here, but this is the raw
+        // error from the underlying exec implementation.  It's not a very
+        // helpful message for users, so it's recommended to craft nice
+        // POJO representation and throw it or promise->reject it.
+        //
+        // TODO:  connection errors may now return JSON on stderr, which we
+        // would attempt to parse here to use a basis for rejection.
+        const { code } = error;
+        return reject({
+          code,
+          message: `OH NOSE!  Command failed with code ${code}`
+        });
+      }
+      return resolve(stdout);
+    });
   });
 });

ui/desktop/electron-app/src/ipc-handler.js

@@ -0,0 +1,35 @@
+const { ipcMain } = require('electron');
+const isDev = require('electron-is-dev');
+
+const log = (method, request='', response='', type='log') => {
+  const requestString = typeof request === 'object'
+    ? JSON.stringify(request)
+    : request;
+  // only log in dev mode
+  if (isDev) {
+    console[type](
+      `[ipc] ${method}(${requestString}): `,
+      type === 'error' ? 'ERROR' : '',
+      response
+    );
+  }
+};
+
+module.exports = function handle (command, handler) {
+  ipcMain.handle(command, async function (event, request) {
+    const requestPayload = request;
+    let result;
+    try {
+      result = await handler(requestPayload);
+      log(command, requestPayload, result);
+    } catch (e) {
+      // If e is an Error instance, we need to extract the message and wrap
+      // it in a POJO before stringifying it.  If not an Error, e is assumed to
+      // be already a POJO.
+      const errorMessage = (e instanceof Error) ? { message: e.message } : e;
+      result = new Error(JSON.stringify(errorMessage));
+      log(command, requestPayload, e, 'error');
+    }
+    return result;
+  });
+};

ui/desktop/tests/unit/services/ipc-test.js

@@ -53,4 +53,26 @@ module('Unit | Service | ipc', function (hooks) {
       test: 42,
     });
   });
+
+  test('IPCRequests reject if receiving an Error instance, where the rejection value is the parsed JSON Error message', async (assert) => {
+    assert.expect(1);
+    const stringifiedPayload = JSON.stringify({
+      foo: 'bar',
+      test: 42
+    });
+    const myWindow = {
+      postMessage(request, origin, [port]) {
+        // Post back an error on the response port
+        port.postMessage(new Error(stringifiedPayload));
+      }
+    };
+    try {
+      await new IPCRequest('hello', 'world', myWindow);
+    } catch (e) {
+      assert.deepEqual(e, {
+        foo: 'bar',
+        test: 42,
+      });
+    }
+  });
 });