Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
62 changes: 57 additions & 5 deletions src/preload/preloadInjection.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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<string, (event: { returnValue?: unknown }) => void>();
Expand All @@ -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);
}
Expand Down Expand Up @@ -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');
});
});
});
45 changes: 43 additions & 2 deletions src/preload/preloadInjection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -76,15 +77,17 @@ export function installBridgePreload(

const registrations = new WeakMap<Session, string>();

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.
Expand All @@ -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
Expand Down
Loading