PR review

Author: timfish

src/common/ipc.ts

@@ -15,8 +15,6 @@ export enum IPCMode {
   Both = 3,
 }
 
-export const PROTOCOL_SCHEME = 'sentry-ipc';
-
 export type Channel =
   /** IPC to check main process is listening */
   | 'start'
@@ -29,30 +27,29 @@ export type Channel =
   /** IPC to pass structured log messages */
   | 'structured-log';
 
-/**
- * Utilities for creating namespaced IPC channels and protocol routes
- */
-export function ipcChannelUtils(namespace: string | undefined): {
+export interface IpcUtils {
   createUrl: (channel: Channel) => string;
   urlMatches: (url: string, channel: Channel) => boolean;
   createKey: (channel: Channel) => string;
-  readonly globalKey: string;
-} {
-  const globalKey = `__${namespace?.replace('-', '_').toUpperCase() || 'SENTRY_IPC'}__`;
+  readonly namespace: string;
+}
 
+/**
+ * Utility for creating namespaced IPC channels and protocol routes
+ */
+export function ipcChannelUtils(namespace: string): IpcUtils {
   return {
     createUrl: (channel: Channel) => {
-      const scheme = namespace ? `${PROTOCOL_SCHEME}-${namespace}` : PROTOCOL_SCHEME;
       // sentry_key in the url stops these messages from being picked up by our HTTP instrumentations
-      return `${scheme}://${channel}/sentry_key`;
+      return `${namespace}://${channel}/sentry_key`;
     },
     urlMatches: function (url: string, channel: Channel): boolean {
       return url.startsWith(this.createUrl(channel));
     },
     createKey: (channel: Channel) => {
-      return namespace ? `${PROTOCOL_SCHEME}-${namespace}.${channel}` : `${PROTOCOL_SCHEME}.${channel}`;
+      return `${namespace}.${channel}`;
     },
-    globalKey,
+    namespace,
   };
 }
 
@@ -109,7 +106,7 @@ export function getMagicMessage(): unknown {
  */
 declare global {
   interface Window {
-    [key: string]: unknown;
+    __SENTRY_IPC__?: Record<string, IPCInterface>;
     __SENTRY__RENDERER_INIT__?: boolean;
     __SENTRY_RENDERER_ID__?: string;
   }

src/main/ipc.ts

@@ -12,7 +12,7 @@ import {
 import { captureEvent, getClient, getCurrentScope } from '@sentry/node';
 import { app, ipcMain, protocol, WebContents, webContents } from 'electron';
 import { eventFromEnvelope } from '../common/envelope.js';
-import { ipcChannelUtils, IPCMode, PROTOCOL_SCHEME, RendererStatus } from '../common/ipc.js';
+import { ipcChannelUtils, IPCMode, IpcUtils, RendererStatus } from '../common/ipc.js';
 import { registerProtocol } from './electron-normalize.js';
 import { createRendererEventLoopBlockStatusHandler } from './integrations/renderer-anr.js';
 import { rendererProfileFromIpc } from './integrations/renderer-profiling.js';
@@ -24,11 +24,6 @@ import { SDK_VERSION } from './version.js';
 let KNOWN_RENDERERS: Set<number> | undefined;
 let WINDOW_ID_TO_WEB_CONTENTS: Map<string, number> | undefined;
 
-const SENTRY_CUSTOM_SCHEME = {
-  scheme: PROTOCOL_SCHEME,
-  privileges: { bypassCSP: true, corsEnabled: true, supportFetchAPI: true, secure: true },
-};
-
 function newProtocolRenderer(): void {
   KNOWN_RENDERERS = KNOWN_RENDERERS || new Set();
   WINDOW_ID_TO_WEB_CONTENTS = WINDOW_ID_TO_WEB_CONTENTS || new Map();
@@ -183,20 +178,23 @@ function handleLogFromRenderer(client: Client, options: ElectronMainOptionsInter
 }
 
 /** Enables Electron protocol handling */
-function configureProtocol(client: Client, options: ElectronMainOptionsInternal): void {
+function configureProtocol(client: Client, ipcUtil: IpcUtils, options: ElectronMainOptionsInternal): void {
   if (app.isReady()) {
     throw new Error("Sentry SDK should be initialized before the Electron app 'ready' event is fired");
   }
 
-  const ipcUtil = ipcChannelUtils(options.ipcNamespace);
+  const scheme = {
+    scheme: ipcUtil.namespace,
+    privileges: { bypassCSP: true, corsEnabled: true, supportFetchAPI: true, secure: true },
+  };
 
-  protocol.registerSchemesAsPrivileged([SENTRY_CUSTOM_SCHEME]);
+  protocol.registerSchemesAsPrivileged([scheme]);
 
   // We Proxy this function so that later user calls to registerSchemesAsPrivileged don't overwrite our custom scheme
   // eslint-disable-next-line @typescript-eslint/unbound-method
   protocol.registerSchemesAsPrivileged = new Proxy(protocol.registerSchemesAsPrivileged, {
     apply: (target, __, args: Parameters<typeof protocol.registerSchemesAsPrivileged>) => {
-      target([...args[0], SENTRY_CUSTOM_SCHEME]);
+      target([...args[0], scheme]);
     },
   });
 
@@ -206,7 +204,7 @@ function configureProtocol(client: Client, options: ElectronMainOptionsInternal)
     .whenReady()
     .then(() => {
       for (const sesh of options.getSessions()) {
-        registerProtocol(sesh.protocol, PROTOCOL_SCHEME, (request) => {
+        registerProtocol(sesh.protocol, ipcUtil.namespace, (request) => {
           const getWebContents = (): WebContents | undefined => {
             const webContentsId = request.windowId ? WINDOW_ID_TO_WEB_CONTENTS?.get(request.windowId) : undefined;
             return webContentsId ? webContents.fromId(webContentsId) : undefined;
@@ -237,9 +235,7 @@ function configureProtocol(client: Client, options: ElectronMainOptionsInternal)
 /**
  * Hooks IPC for communication with the renderer processes
  */
-function configureClassic(client: Client, options: ElectronMainOptionsInternal): void {
-  const ipcUtil = ipcChannelUtils(options.ipcNamespace);
-
+function configureClassic(client: Client, ipcUtil: IpcUtils, options: ElectronMainOptionsInternal): void {
   ipcMain.on(ipcUtil.createKey('start'), ({ sender }) => {
     const id = sender.id;
     // Keep track of renderers that are using IPC
@@ -276,13 +272,15 @@ function configureClassic(client: Client, options: ElectronMainOptionsInternal):
 
 /** Sets up communication channels with the renderer */
 export function configureIPC(client: Client, options: ElectronMainOptionsInternal): void {
+  const ipcUtil = ipcChannelUtils(options.ipcNamespace);
+
   // eslint-disable-next-line no-bitwise
   if ((options.ipcMode & IPCMode.Protocol) > 0) {
-    configureProtocol(client, options);
+    configureProtocol(client, ipcUtil, options);
   }
 
   // eslint-disable-next-line no-bitwise
   if ((options.ipcMode & IPCMode.Classic) > 0) {
-    configureClassic(client, options);
+    configureClassic(client, ipcUtil, options);
   }
 }

src/main/sdk.ts

@@ -107,9 +107,11 @@ export interface ElectronMainOptionsInternal
    * Custom namespace for IPC channels and protocol routes.
    *
    * Valid characters are a-z, 0-9, hyphen (-).
-   * Should match `ipcNamespace` passed in renderer init.
+   * Should match `ipcNamespace` passed in the renderer processes.
+   *
+   * @default 'sentry-ipc'
    */
-  ipcNamespace?: string;
+  ipcNamespace: string;
 
   /**
    * A function that returns an array of Electron session objects
@@ -145,8 +147,11 @@ export interface ElectronMainOptionsInternal
 }
 
 // getSessions and ipcMode properties are optional because they have defaults
-export type ElectronMainOptions = Pick<Partial<ElectronMainOptionsInternal>, 'getSessions' | 'ipcMode'> &
-  Omit<ElectronMainOptionsInternal, 'getSessions' | 'ipcMode'> &
+export type ElectronMainOptions = Pick<
+  Partial<ElectronMainOptionsInternal>,
+  'getSessions' | 'ipcMode' | 'ipcNamespace'
+> &
+  Omit<ElectronMainOptionsInternal, 'getSessions' | 'ipcMode' | 'ipcNamespace'> &
   NodeOptions;
 
 /**
@@ -162,6 +167,7 @@ export function init(userOptions: ElectronMainOptions): void {
   const optionsWithDefaults = {
     _metadata: { sdk: getSdkInfo(!!userOptions.sendDefaultPii) },
     ipcMode: IPCMode.Both,
+    ipcNamespace: 'sentry-ipc',
     release: getDefaultReleaseName(),
     environment: getDefaultEnvironment(),
     defaultIntegrations: getDefaultIntegrations(userOptions),

src/preload/index.ts

@@ -11,11 +11,14 @@ import { ipcChannelUtils, RendererStatus } from '../common/ipc.js';
  *
  * @param namespace An optional namespace to use for the IPC channels
  */
-export function hookupIpc(namespace?: string): void {
+export function hookupIpc(namespace: string = 'sentry-ipc'): void {
   const ipcUtil = ipcChannelUtils(namespace);
 
   // eslint-disable-next-line no-restricted-globals
-  if (window[ipcUtil.globalKey]) {
+  window.__SENTRY_IPC__ = window.__SENTRY_IPC__ || {};
+
+  // eslint-disable-next-line no-restricted-globals
+  if (window.__SENTRY_IPC__[ipcUtil.namespace]) {
     // eslint-disable-next-line no-console
     console.log('Sentry Electron preload has already been run');
   } else {
@@ -28,13 +31,13 @@ export function hookupIpc(namespace?: string): void {
     };
 
     // eslint-disable-next-line no-restricted-globals
-    window[ipcUtil.globalKey] = ipcObject;
+    window.__SENTRY_IPC__[ipcUtil.namespace] = ipcObject;
 
     // We attempt to use contextBridge if it's available
     if (contextBridge) {
       // This will fail if contextIsolation is not enabled
       try {
-        contextBridge.exposeInMainWorld(ipcUtil.globalKey, ipcObject);
+        contextBridge.exposeInMainWorld(ipcUtil.namespace, ipcObject);
       } catch (e) {
         //
       }

src/renderer/ipc.ts

@@ -2,15 +2,17 @@
 /* eslint-disable no-console */
 import { Client, debug, getClient, SerializedLog, uuid4 } from '@sentry/core';
 import { ipcChannelUtils, IPCInterface, RENDERER_ID_HEADER, RendererStatus } from '../common/ipc.js';
-import { ElectronRendererOptions } from './sdk.js';
+import { ElectronRendererOptionsInternal } from './sdk.js';
 
 /** Gets the available IPC implementation */
-function getImplementation(ipcKey: string | undefined): IPCInterface {
+function getImplementation(ipcKey: string): IPCInterface {
   const ipcUtil = ipcChannelUtils(ipcKey);
 
+  window.__SENTRY_IPC__ = window.__SENTRY_IPC__ || {};
+
   // Favour IPC if it's been exposed by a preload script
-  if (window[ipcUtil.globalKey]) {
-    return window[ipcUtil.globalKey] as IPCInterface;
+  if (window.__SENTRY_IPC__[ipcUtil.namespace]) {
+    return window.__SENTRY_IPC__[ipcUtil.namespace] as IPCInterface;
   } else {
     debug.log('IPC was not configured in preload script, falling back to custom protocol and fetch');
 
@@ -83,7 +85,7 @@ export function getIPC(client: Client | undefined = getClient()): IPCInterface {
     return found;
   }
 
-  const namespace = (client.getOptions() as ElectronRendererOptions).ipcNamespace;
+  const namespace = (client.getOptions() as ElectronRendererOptionsInternal).ipcNamespace;
   const implementation = getImplementation(namespace);
   cachedInterfaces.set(client, implementation);
   implementation.sendRendererStart();

src/renderer/sdk.ts

@@ -18,7 +18,7 @@ export function getDefaultIntegrations(options: ElectronRendererOptions): Integr
   ];
 }
 
-export interface ElectronRendererOptions extends Omit<BrowserOptions, 'dsn' | 'environment' | 'release'> {
+export interface ElectronRendererOptionsInternal extends Omit<BrowserOptions, 'dsn' | 'environment' | 'release'> {
   /** @deprecated `dsn` should only be passed to the main process `Sentry.init` call */
   dsn?: string;
   /** @deprecated `release` should only be passed to the main process `Sentry.init` call */
@@ -31,8 +31,15 @@ export interface ElectronRendererOptions extends Omit<BrowserOptions, 'dsn' | 'e
    *
    * Valid characters are a-z, 0-9, hyphen (-).
    * Should match `ipcNamespace` passed in the main process.
+   *
+   * @default 'sentry-ipc'
    */
-  ipcNamespace?: string;
+  ipcNamespace: string;
+}
+
+// eslint-disable-next-line @typescript-eslint/no-empty-interface
+interface ElectronRendererOptions extends Partial<ElectronRendererOptionsInternal> {
+  //
 }
 
 /**
@@ -67,6 +74,10 @@ If init has been called in the preload and contextIsolation is disabled, is not
     options.stackParser = electronRendererStackParser;
   }
 
+  if (options.ipcNamespace === undefined) {
+    options.ipcNamespace = 'sentry-ipc';
+  }
+
   // eslint-disable-next-line deprecation/deprecation
   if (options.dsn === undefined) {
     // Events are sent via the main process but browser SDK wont start without dsn