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

[RFC] Interprocess v1 #28

Open
6 tasks
daltonmenezes opened this issue Mar 15, 2024 · 7 comments
Open
6 tasks

[RFC] Interprocess v1 #28

daltonmenezes opened this issue Mar 15, 2024 · 7 comments
Assignees
Labels

Comments

@daltonmenezes
Copy link
Owner

daltonmenezes commented Mar 15, 2024

🫡 Hi everyone, these are some of the planned things that will happen in the Interprocess v1 release!
I'll update this issue on any additions/changes for the release!

Feel free to suggest anything pertinent!! 🤗

Features

  • mainSlice - a function that creates an ipc slice for the main process.
    • it replaces the need of createIpcSlice and combineIpcs usage!
       import { mainSlice, createInterprocess } from 'interprocess'
    
       // The slice can be moved to another file or folder for better organization
       const pingSlice = mainSlice(async (_, data: string) => { ... })
       
       export const { ipcMain, ipcRenderer, exposeApiToGlobalWindow } = createInterprocess({
         main: {
           pingSlice,
         }
      })      
  • rendererSlice - a function that creates an ipc slice for the renderer process.
    • same as mainSlice but for renderer
  • middleware/hook support

Docs

  • improve the docs to give more clarity on how to use interprocess in the best way, mainly to extract the best use of ipcs without the need to create dumb ones to only get the types! This has happened because there are no examples of dynamic import in ipcs and this has generated conflicts and misuse of them.

Breaking changes

  • combineIpcs (it will be removed on v1)
  • createIpcSlice (it will be removed on v1)
@daltonmenezes daltonmenezes self-assigned this Mar 15, 2024
@daltonmenezes daltonmenezes pinned this issue Mar 15, 2024
@theogravity
Copy link

theogravity commented Mar 15, 2024

One note: You can't use interprocess via npm package install if you use sandbox: true for the BrowserWindow prefs because the sandbox won't let you use import node modules in the renderer process since:

A sandboxed renderer won't have a Node.js environment initialized.

To get around this, I had to copy the src files directly to my project in a vendor directory.

Other than that, had really great success with interprocess on our electron app for https://switchboard.app

@theogravity
Copy link

theogravity commented Mar 15, 2024

Just updated our interprocess to latest as I was trying to figure out a memory leak around invocations, which the latest version does fix in createMainInvokers.ts:

if (!store.main.availableRendererIpcChannels.has(ipcChannel)) {
       return reject(`No handler registered for '${ipcChannel}'`)
}

I wish there was a silent way to reject as we have tons of IPC calls and it'd be a huge pain to wrap try/catch around them. It definitely shouldn't proceed with the subsequent code in this situation, but we'll encounter the error situation at times if our electron version and frontend doesn't match up (as in the FE may be outdated and doesn't register a handler until a later version). We assumed that we can invoke and it will be a technical no-op in this situation since it has no where to send to.

The "fix" I'm doing is something akin to wrapping the invokers like we do handlers (see next comment) by wrapping try / catch around it.

@theogravity
Copy link

Another feature suggestion is invoker / handler middleware. I have a custom class that wraps the handlers:

  /**
   * This wraps the IPC handlers with another method that will auto-cleanup the handler if it should be
   * re-created again
   */
  private wrapHandlers() {
    this.ipc['handle'] = Object.keys(this.ipc['handle']).reduce((handlers, handlerName) => {
      const handlerFn = this.ipc['handle'][handlerName];

      handlers[handlerName] = (cb: HandlerCallback) => {
        const checkSenderCallback = this.checkSenderOriginMiddleware(handlerName, cb);
        this.cleanupHandler(handlerName);
        return handlerFn(checkSenderCallback);
      };

      return handlers;
    }, {});
  }

The reason for this is because of security - we apply an origin check for any IPC calls from the FE to electron before we proceed with execution. If interprocess had some means to use middleware, this kind of hack wouldn't be necessary.

@theogravity
Copy link

here's what I did for the invokers

  /**
   * Wraps a try / catch around the invokers to ignore errors around the corresponding handler not
   * being registered (this could be due to an older FE version) when the invoker is called.
   */
  private wrapInvokers() {
    this.ipc['invoke'] = Object.keys(this.ipc['invoke']).reduce((invokers, invokeName) => {
      const invokeFn = this.ipc['invoke'][invokeName];

      invokers[invokeName] = async (window: BrowserWindow, ...args: any[]) => {
        try {
          return await invokeFn(window, ...args);
        } catch (e) {
          // It's ok if we have no handler registered - the FE might be an older version
          // and doesn't register the handler so we ignore this error
          if (typeof e === 'string' && !e.includes('No handler registered for')) {
            this.log.debug(`Error invoking ${invokeName}: ${e}`);
          } else if (e instanceof Error) {
            this.log.error(`Error invoking ${invokeName}: ${e.message}`);
          }
        }
      };

      return invokers;
    }, {});
  }
  

@daltonmenezes
Copy link
Owner Author

Hi @theogravity

One note: You can't use interprocess via npm package install if you use sandbox: true for the BrowserWindow prefs because the sandbox won't let you use import node modules in the renderer process since:

A sandboxed renderer won't have a Node.js environment initialized.

To get around this, I had to copy the src files directly to my project in a vendor directory.

Other than that, had really great success with interprocess on our electron app for https://switchboard.app

The docs mention the need of sandbox: false, but I'm thinking what's the best approach in the cases where sandbox: true are not negotiable. Maybe we just have to mention in the docs how to make it work like you did? Any ideas?

About the Switchboard, it really looks awesome, congrats!

Just updated our interprocess to latest as I was trying to figure out a memory leak around invocations, which the latest version does fix in createMainInvokers.ts:

if (!store.main.availableRendererIpcChannels.has(ipcChannel)) {
       return reject(`No handler registered for '${ipcChannel}'`)
}

I wish there was a silent way to reject as we have tons of IPC calls and it'd be a huge pain to wrap try/catch around them. It definitely shouldn't proceed with the subsequent code in this situation, but we'll encounter the error situation at times if our electron version and frontend doesn't match up (as in the FE may be outdated and doesn't register a handler until a later version). We assumed that we can invoke and it will be a technical no-op in this situation since it has no where to send to.

The "fix" I'm doing is something akin to wrapping the invokers like we do handlers (see next comment) by wrapping try / catch around it.

About the rejection, I believe this must be the default behavior, but maybe we should have unsafe handle/invoke methods to not throw? (good to use-as-needed) like zod does in safeParse (I really don't like naming something like zod calling something as safe and not throwing, unsafe is less confusing, right? What do you prefer?)

Another feature suggestion is invoker / handler middleware. I have a custom class that wraps the handlers:

  /**
   * This wraps the IPC handlers with another method that will auto-cleanup the handler if it should be
   * re-created again
   */
  private wrapHandlers() {
    this.ipc['handle'] = Object.keys(this.ipc['handle']).reduce((handlers, handlerName) => {
      const handlerFn = this.ipc['handle'][handlerName];

      handlers[handlerName] = (cb: HandlerCallback) => {
        const checkSenderCallback = this.checkSenderOriginMiddleware(handlerName, cb);
        this.cleanupHandler(handlerName);
        return handlerFn(checkSenderCallback);
      };

      return handlers;
    }, {});
  }

The reason for this is because of security - we apply an origin check for any IPC calls from the FE to electron before we proceed with execution. If interprocess had some means to use middleware, this kind of hack wouldn't be necessary.

I really like the idea of having middlewares/hooks support on interprocess! 💜
I'll be thinking of a good way to make this happen from a DX perspective!

By the way, thank you very much for the feedback and sponsorship! 💜

@theogravity
Copy link

theogravity commented Mar 15, 2024

I don't really know what would be good DX for the sandbox: true case, but maybe you can have an npx command that would just copy the src dir (along with the README.md and LICENSE files) to a directory of the user's choosing?

Regarding the safe proposition, maybe it can just be a config option when calling createInterprocess. When enabled, it'd empty-resolve (not like it matters since there's nothing to handle it anyways) on an invoke without a handler.

@theogravity
Copy link

theogravity commented Mar 15, 2024

The IPC base class we use to create categories of IPCs might help give you ideas:

// in main process
interface IpcHandlers {
  remove: Record<string, () => void>;
}

type HandlerCallback = (event: IpcMainInvokeEvent, params: any) => Promise<any>;

export interface IpcHandlersParams<T extends IpcHandlers> {
  logPrefix: string;
  ipc: T;
}

/**
 * All IPC handlers extend this class.
 */
export abstract class BaseIpcHandlers<T extends IpcHandlers> {
  /**
   * Set of registered channel names
   */
  private registeredHandlers: Set<string>;
  /**
   * The IPC instance (*ipcMain) from interprocess
   * @protected
   */
  protected ipc: T;

  log: LogLayer;

  protected constructor(params: IpcHandlersParams<T>) {
    this.registeredHandlers = new Set();
    this.log = getMainLogger().withPrefix(`[${params.logPrefix}IPC]`);
    this.ipc = params.ipc;
    this.wrapHandlers();
    this.wrapInvokers();
  }

  abstract initGlobalHandlers(): Promise<void>;

  /**
   * This wraps the IPC handlers with another method that will auto-cleanup the handler if it should be
   * re-created again
   */
  private wrapHandlers() {
    this.ipc['handle'] = Object.keys(this.ipc['handle']).reduce((handlers, handlerName) => {
      const handlerFn = this.ipc['handle'][handlerName];

      handlers[handlerName] = (cb: HandlerCallback) => {
        const checkSenderCallback = this.checkSenderOriginMiddleware(handlerName, cb);
        this.cleanupHandler(handlerName);
        return handlerFn(checkSenderCallback);
      };

      return handlers;
    }, {});
  }

  /**
   * Wraps a try / catch around the invokers to ignore errors around the corresponding handler not
   * being registered (this could be due to an older FE version) when the invoker is called.
   */
  private wrapInvokers() {
    this.ipc['invoke'] = Object.keys(this.ipc['invoke']).reduce((invokers, invokeName) => {
      const invokeFn = this.ipc['invoke'][invokeName];

      invokers[invokeName] = async (window: BrowserWindow, ...args: any[]) => {
        try {
          return await invokeFn(window, ...args);
        } catch (e) {
          // It's ok if we have no handler registered - the FE might be an older version
          // and doesn't register the handler so we ignore this error
          if (typeof e === 'string' && !e.includes('No handler registered for')) {
            this.log.debug(`Error invoking ${invokeName}: ${e}`);
          } else if (e instanceof Error) {
            this.log.error(`Error invoking ${invokeName}: ${e.message}`);
          }
        }
      };

      return invokers;
    }, {});
  }

  /**
   * Takes in the original IPC handler function and wraps it with an origin check before calling
   * the original IPC handler function
   */
  private checkSenderOriginMiddleware(handlerName: string, callback: HandlerCallback): HandlerCallback {
    // ...
  }

  /**
   * This registers the handler for auto-removal in the event
   * that the handler is re-registered since handlers can only be
   * registered once at a time.
   */
  private cleanupHandler(handlerName: string) {
    if (this.registeredHandlers.has(handlerName)) {
      this.unregisterHandler(handlerName);
    }

    this.registeredHandlers.add(handlerName);
  }

  private unregisterHandler(handlerName: string) {
    const canRemove = this.registeredHandlers.has(handlerName);

    if (canRemove) {
      this.registeredHandlers.delete(handlerName);
      // This calls ipcMain/Renderer.remove.<name>
      if (this.ipc.remove[handlerName]) {
        this.ipc.remove[handlerName]();
      }
    }
  }
}

An example impl:

// in main process
export class AppIpcHandlers extends BaseIpcHandlers<typeof appIpcMain> {
  constructor() {
    super({
      logPrefix: 'App',
      // from createInterprocess
      ipc: appIpcMain,
    });

    // registers the handler when we start the app up
    this.bounceApp();
  }

  // this is if we need to register the handlers in an async fashion
  async initGlobalHandlers() {
    return;
  }

  bounceApp() {
    this.ipc.handle.bounceApp(async (_, { data: bounceType }) => {
      if (bounceType === 'critical' || bounceType === 'informational') {
        app.dock.bounce(bounceType);
      }
    });
  }
}

We create these classes and call await initGlobalHandlers() in our main/index.ts during app init

Note that we don't define logic in the IPC def itself so the typescript typing can work between main and renderer:

// defined outside of main / renderer so it can be imported into both
export const { ipcMain: appIpcMain, ipcRenderer: appIpcRenderer } = createInterprocess({
  main: {
    /**
     * Bounces the app icon on the dock on mac.
     */
    async bounceApp(_, _type: 'critical' | 'informational') {
      return;
    },
  },
  renderer: {
    /**
     * The user has clicked something.
     */
    async onClick() {
      return;
    },
  },
});

For the renderer items, we create classes for bridge connection:

// in preload
export class AppBridge implements AppNativeAPI {
  static bridgeName = 'app';
  sendOnClickCallback: boolean;

  constructor() {
    this.sendOnClickCallback = false;
  }

  async bounceApp(type) {
    return appIpcRenderer.invoke.bounceApp(type);
  }

  onClick(callback) {
    // We can only have a single handler at a time
    if (this.sendOnClickCallback) {
      appIpcRenderer.remove.onClick();
    }

    appIpcRenderer.handle.onClick(async () => {
      await callback();
    });

    this.sendOnClickCallback = true;
  }
}
// preload

/**
 * Takes a bridge class definition and extracts the methods out of it
 * into an object that can be consumed by contextBridge.exposeInMainWorld
 * as contextBridge.exposeInMainWorld cannot accept anything other than raw
 * objects and functions
 */
export function extractBridgeMethods<T>(ClassDef): T {
  const classInstance = new ClassDef();
  const exported = {};

  for (const name of Object.getOwnPropertyNames(Object.getPrototypeOf(classInstance))) {
    const method = classInstance[name];
    // Supposedly you'd like to skip constructor
    if (!(method instanceof Function) || method === ClassDef) continue;
    exported[name] = method;
  }

  return exported as T;
}

const api: MainNativeAPI = {
  // Increment this when something changes
  API_LEVEL: 2,
  app: extractBridgeMethods<AppBridge>(AppBridge),
};

contextBridge.exposeInMainWorld('someVar', api);

@daltonmenezes daltonmenezes changed the title Interprocess v1 [RFC] Interprocess v1 Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants