diff --git a/CHANGELOG.md b/CHANGELOG.md index 155b0ce5..854961e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ All notable changes to `@flashcatcloud/electron-sdk` are documented here. +## [0.2.2] + +### 🐛 Bug Fixes + +- An Electron older than 35 no longer stops the application from starting. `session.registerPreloadScript` was introduced in Electron 35, and the SDK registers the bridge preload through it from an `app` 'ready' listener and from 'session-created'. On an older runtime the missing method threw where the host application had no way to catch it, and since the instrumentation entry point runs before the application's own 'ready' handler, the failure preempted window creation: the application died at startup with `Cannot read properties of undefined (reading 'bind')`, a message that says nothing about the Electron version behind it. `peerDependencies` did not prevent this — it is an installation error only under npm, a warning under Yarn and pnpm. + + Sessions on such a runtime now get a stand-in that accepts preload registrations and drops them, and a single warning names the cause. The renderer bridge is what an unsupported Electron costs: main process monitoring is unaffected and the Browser SDK keeps collecting in renderers, though renderer events do not share the main process session. The listeners the SDK puts on the host application are wrapped as well, so no failure inside them can propagate into it. + ## [0.2.1] ### 🐛 Bug Fixes diff --git a/package.json b/package.json index 41388816..b2777d26 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/electron-sdk", - "version": "0.2.1", + "version": "0.2.2", "description": "FlashCat SDK for Electron applications", "packageManager": "yarn@4.12.0", "main": "./dist/index.cjs", diff --git a/src/preload/preloadInjection.spec.ts b/src/preload/preloadInjection.spec.ts index 98f92d09..f0c63bce 100644 --- a/src/preload/preloadInjection.spec.ts +++ b/src/preload/preloadInjection.spec.ts @@ -19,14 +19,16 @@ interface FakeSession { registered: string[]; } -function createSession(): FakeSession { +function createSession({ legacy = false } = {}): FakeSession { const registered: string[] = []; let nextId = 0; const registerPreloadScript = vi.fn((script: PreloadScriptRegistration) => { registered.push(script.filePath); return `registration-${++nextId}`; }); - return { session: { registerPreloadScript } as unknown as Session, registerPreloadScript, registered }; + // Electron < 35 has no `registerPreloadScript` on Session at all. + const session = (legacy ? {} : { registerPreloadScript }) as unknown as Session; + return { session, registerPreloadScript, registered }; } interface FakeHost { @@ -38,8 +40,8 @@ interface FakeHost { requestConfig: (channel: string) => unknown; } -function createHost({ readyUpfront = false } = {}): FakeHost { - const defaultSession = createSession(); +function createHost({ readyUpfront = false, legacy = false } = {}): FakeHost { + const defaultSession = createSession({ legacy }); const sessionListeners: ((session: Session) => void)[] = []; const readyListeners: (() => void)[] = []; const ipcListeners = new Map void>(); @@ -63,7 +65,7 @@ function createHost({ readyUpfront = false } = {}): FakeHost { return ipcEvent.returnValue; }, createSession: () => { - const created = createSession(); + const created = createSession({ legacy }); for (const listener of sessionListeners) { listener(created.session); } @@ -242,4 +244,54 @@ describe('installBridgePreload', () => { expect(requestConfig(CONFIG_CHANNEL)).toBeUndefined(); }); }); + + /** + * Electron < 35 has no `session.registerPreloadScript`. Both this SDK and dd-trace's + * `BrowserWindow` subclass call it unconditionally, from places the host application cannot + * guard, so its absence must cost monitoring — never startup. + */ + describe('an Electron without registerPreloadScript', () => { + it('should not throw out of the app ready listener', async () => { + const { host, emitReady } = createHost({ legacy: true }); + await install(host); + + expect(emitReady).not.toThrow(); + }); + + it('should not throw out of the session-created listener', async () => { + const { host, createSession: createNewSession } = createHost({ legacy: true }); + await install(host); + + expect(createNewSession).not.toThrow(); + }); + + it('should not throw when the app is ready before the SDK installs', async () => { + const { host } = createHost({ legacy: true, readyUpfront: true }); + + await expect(install(host)).resolves.toBeUndefined(); + }); + + it('should leave a dd-trace registration harmless, so windows can still be created', async () => { + const { host, defaultSession, emitReady } = createHost({ legacy: true }); + await install(host); + emitReady(); + + // What dd-trace's BrowserWindow subclass does on every window it creates. + expect(() => + defaultSession.session.registerPreloadScript({ type: 'frame', filePath: DD_TRACE_PRELOAD_PATH }) + ).not.toThrow(); + }); + + it('should say why once, not once per session', async () => { + const { host, emitReady, createSession: createNewSession } = createHost({ legacy: true }); + await install(host); + emitReady(); + createNewSession(); + createNewSession(); + + const { displayWarn } = await import('../tools/display'); + expect(displayWarn).toHaveBeenCalledTimes(1); + expect(vi.mocked(displayWarn).mock.calls[0][0]).toContain('Electron 35'); + }); + }); }); diff --git a/src/preload/preloadInjection.ts b/src/preload/preloadInjection.ts index 2bf2d1e7..c6f38e65 100644 --- a/src/preload/preloadInjection.ts +++ b/src/preload/preloadInjection.ts @@ -2,6 +2,7 @@ import { createRequire } from 'node:module'; import { existsSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { monitor } from '@flashcatcloud/browser-core/cjs/tools/monitor'; import type { IpcMainEvent, PreloadScriptRegistration, Session } from 'electron'; import { displayWarn } from '../tools/display'; import { CONFIG_CHANNEL } from '../common'; @@ -76,15 +77,17 @@ export function installBridgePreload( const registrations = new WeakMap(); - const setUp = (session: Session): void => { + const setUp = monitor((session: Session): void => { if (registrations.has(session)) { return; } + standInForMissingRegisterPreloadScript(session); + const register = registerOnce(session, preloadPath, registrations); takeOverDdTraceRegistrations(session, register); register(); - }; + }); // Sessions are created before the windows using them, so a session-created listener always runs // before dd-trace's BrowserWindow subclass gets a chance to register its own preload. @@ -100,6 +103,44 @@ export function installBridgePreload( } } +let warnedMissingRegisterPreloadScript = false; + +/** + * `session.registerPreloadScript` was added in Electron 35, and older versions do reach this code: + * `peerDependencies` is an installation error only under npm, a warning under Yarn and pnpm. The + * SDK registers the bridge preload from an `app` 'ready' listener and from 'session-created', + * neither of which the host application can guard, so the method's absence stops the application + * from starting rather than costing it monitoring. + * + * dd-trace registers a preload of its own the same way, but `datadog-instrumentations` gates its + * whole Electron hook on `electron >= 37`, so below that there is nothing of its to take over. + * + * Give such a session a stand-in that accepts registrations and drops them. Losing the renderer + * bridge is what an unsupported Electron costs; the application still starts, and main process + * monitoring is unaffected. + */ +function standInForMissingRegisterPreloadScript(session: Session): void { + if (typeof session.registerPreloadScript === 'function') { + return; + } + + if (!warnedMissingRegisterPreloadScript) { + warnedMissingRegisterPreloadScript = true; + displayWarn( + 'This Electron version has no session.registerPreloadScript (added in Electron 35), so the ' + + 'renderer bridge cannot be installed. Main process monitoring is unaffected and the Browser ' + + 'SDK keeps collecting in renderers, but renderer events will not share the main process ' + + 'session. Upgrade to Electron 35 or later to restore the bridge.' + ); + } + + try { + session.registerPreloadScript = () => ''; + } catch (error) { + displayWarn('Failed to install the preload registration stand-in:', error); + } +} + /** * Register the preload on a session at most once, whatever the number of callers. * `registerPreloadScript` is cumulative and session-wide: calling it again adds a registration