Skip to content

Commit 6dee197

Browse files
authored
fix(electron): ensure valid values cross bridge (#1256)
This PR will surround the IPC message send with a try catch in case someone attempts to send a non-serializable. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes renderer↔main IPC behavior by catching `sendSync` serialization failures and returning fallbacks, which could mask errors or change returned values in edge cases. Also adjusts Electron version constraints, which may affect downstream compatibility. > > **Overview** > Improves Electron renderer→main IPC robustness by wrapping several `ipcRenderer.sendSync` calls (`variation*`, `jsonVariation*`, `track`) in `safeSendSync`, returning a fallback value when IPC serialization/clone fails and emitting a warning via a new `log` IPC channel. > > Adds main-process handling for the new `log` channel in `ElectronClient` (dispatching to `logger` for `error|warn|info|debug` only) and updates tests to cover fallback behavior and invalid log levels. Updates Electron dependency/peer dependency constraints (SDK peer dep now `electron >=34.5.6`, dev/example use `^40.2.1`). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit c6e89f1. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/launchdarkly/js-core/pull/1256" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end -->
1 parent aa8d79d commit 6dee197

5 files changed

Lines changed: 103 additions & 6 deletions

File tree

packages/sdk/electron/__tests__/ElectronClient.ipcMain.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,28 @@ describe('given an initialized ElectronClient', () => {
167167
expect(result).toEqual({ status: 'completed' });
168168
});
169169

170+
it('handles log() by dispatching to the correct logger method', () => {
171+
mockIpcMain.getHandler(getEventName('log'))?.({}, 'warn', 'test warning');
172+
expect(logger.warn).toHaveBeenCalledWith('test warning');
173+
174+
mockIpcMain.getHandler(getEventName('log'))?.({}, 'error', 'test error');
175+
expect(logger.error).toHaveBeenCalledWith('test error');
176+
177+
mockIpcMain.getHandler(getEventName('log'))?.({}, 'info', 'test info');
178+
expect(logger.info).toHaveBeenCalledWith('test info');
179+
180+
mockIpcMain.getHandler(getEventName('log'))?.({}, 'debug', 'test debug');
181+
expect(logger.debug).toHaveBeenCalledWith('test debug');
182+
});
183+
184+
it('ignores log() calls with invalid severity levels', () => {
185+
mockIpcMain.getHandler(getEventName('log'))?.({}, 'invalid', 'should be ignored');
186+
expect(logger.warn).not.toHaveBeenCalled();
187+
expect(logger.error).not.toHaveBeenCalled();
188+
expect(logger.info).not.toHaveBeenCalled();
189+
expect(logger.debug).not.toHaveBeenCalled();
190+
});
191+
170192
it('handles jsonVariation() call', () => {
171193
const expected = { key1: 'value', key2: true };
172194

packages/sdk/electron/__tests__/bridge/LDClientBridge.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ jest.mock('electron', () => ({
1414
}),
1515
},
1616
ipcRenderer: {
17+
send: jest.fn(),
1718
sendSync: jest.fn(),
1819
invoke: jest.fn(),
1920
postMessage: jest.fn(),
@@ -360,6 +361,36 @@ describe('given a registered LDClientBridge', () => {
360361
expect(result).toEqual(true);
361362
});
362363

364+
it('sends log warning and returns fallback when sendSync throws', () => {
365+
const error = new Error('Could not clone');
366+
(ipcRenderer.sendSync as jest.Mock).mockImplementationOnce(() => {
367+
throw error;
368+
});
369+
370+
const defaultValue = { key: 'default' };
371+
const result = bridge.jsonVariation('flag1', defaultValue);
372+
373+
expect(result).toEqual(defaultValue);
374+
expect(ipcRenderer.send).toHaveBeenCalledWith(
375+
getEventName('log'),
376+
'warn',
377+
expect.stringContaining('Could not clone'),
378+
);
379+
});
380+
381+
it('returns well-formed detail on error for variationDetail', () => {
382+
(ipcRenderer.sendSync as jest.Mock).mockImplementationOnce(() => {
383+
throw new Error('clone failed');
384+
});
385+
386+
const result = bridge.variationDetail('flag1', 'fallback');
387+
388+
expect(result).toEqual({
389+
value: 'fallback',
390+
reason: { kind: 'ERROR', errorKind: 'EXCEPTION' },
391+
});
392+
});
393+
363394
it('invokes optional onClose when the message port is closed', () => {
364395
const callback = jest.fn();
365396
const onClose = jest.fn();

packages/sdk/electron/src/ElectronClient.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
LDHeaders,
1919
LDIdentifyOptions,
2020
LDIdentifyResult,
21+
LDLogger,
2122
LDPluginEnvironmentMetadata,
2223
LDWaitForInitializationOptions,
2324
LDWaitForInitializationResult,
@@ -39,6 +40,8 @@ import type { LDPlugin } from './LDPlugin';
3940
import validateOptions, { filterToBaseOptions } from './options';
4041
import ElectronPlatform from './platform/ElectronPlatform';
4142

43+
const VALID_LOG_LEVELS: ReadonlySet<string> = new Set(['error', 'warn', 'info', 'debug']);
44+
4245
export class ElectronClient extends LDClientImpl {
4346
private readonly _initialContext: LDContext;
4447

@@ -286,6 +289,12 @@ export class ElectronClient extends LDClientImpl {
286289
this.identifyResult(context, identifyOptions),
287290
);
288291

292+
ipcMain.on(getIPCChannelName(credential, 'log'), (_event, level: string, message: string) => {
293+
if (VALID_LOG_LEVELS.has(level)) {
294+
this.logger[level as keyof LDLogger](message);
295+
}
296+
});
297+
289298
ipcMain.on(getIPCChannelName(credential, 'jsonVariation'), (event, key, defaultValue) => {
290299
// eslint-disable-next-line no-param-reassign
291300
event.returnValue = this.jsonVariation(key, defaultValue);

packages/sdk/electron/src/ElectronIPC.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ type IPCSyncChannel =
1212
| 'getContext'
1313
| 'jsonVariation'
1414
| 'jsonVariationDetail'
15+
| 'log'
1516
| 'numberVariation'
1617
| 'numberVariationDetail'
1718
| 'stringVariation'
@@ -36,6 +37,7 @@ export const AllSyncChannels: readonly IPCSyncChannel[] = [
3637
'getContext',
3738
'jsonVariation',
3839
'jsonVariationDetail',
40+
'log',
3941
'numberVariation',
4042
'numberVariationDetail',
4143
'stringVariation',

packages/sdk/electron/src/bridge/index.ts

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type {
1414
LDWaitForInitializationResult,
1515
} from '@launchdarkly/js-client-sdk-common';
1616

17+
import type { IPCChannel } from '../ElectronIPC';
1718
import { getIPCChannelName } from '../ElectronIPC';
1819
import type { LDClientBridge, LDMessagePort } from './LDClientBridge';
1920

@@ -23,6 +24,25 @@ import type { LDClientBridge, LDMessagePort } from './LDClientBridge';
2324
const generateCallbackId = () =>
2425
`${Date.now().toString(36)}${Math.random().toString(36).substring(2)}`.toUpperCase();
2526

27+
function safeSendSync<T>(
28+
namespace: string,
29+
method: IPCChannel,
30+
fallback: T,
31+
...args: unknown[]
32+
): T {
33+
try {
34+
return ipcRenderer.sendSync(getIPCChannelName(namespace, method), ...args);
35+
} catch (e: unknown) {
36+
const message = e instanceof Error ? e.message : String(e);
37+
ipcRenderer.send(
38+
getIPCChannelName(namespace, 'log'),
39+
'warn',
40+
`${method}: a value is not IPC-serializable and will be dropped. ${message}`,
41+
);
42+
return fallback;
43+
}
44+
}
45+
2646
const ldClientBridge = (namespace: string): LDClientBridge => ({
2747
allFlags: (): LDFlagSet => ipcRenderer.sendSync(getIPCChannelName(namespace, 'allFlags')),
2848

@@ -43,10 +63,16 @@ const ldClientBridge = (namespace: string): LDClientBridge => ({
4363
ipcRenderer.invoke(getIPCChannelName(namespace, 'identify'), context, identifyOptions),
4464

4565
jsonVariation: (key: string, defaultValue: unknown): unknown =>
46-
ipcRenderer.sendSync(getIPCChannelName(namespace, 'jsonVariation'), key, defaultValue),
66+
safeSendSync<unknown>(namespace, 'jsonVariation', defaultValue, key, defaultValue),
4767

4868
jsonVariationDetail: (key: string, defaultValue: unknown): LDEvaluationDetailTyped<unknown> =>
49-
ipcRenderer.sendSync(getIPCChannelName(namespace, 'jsonVariationDetail'), key, defaultValue),
69+
safeSendSync<LDEvaluationDetailTyped<unknown>>(
70+
namespace,
71+
'jsonVariationDetail',
72+
{ value: defaultValue, reason: { kind: 'ERROR', errorKind: 'EXCEPTION' } },
73+
key,
74+
defaultValue,
75+
),
5076

5177
waitForInitialization: (
5278
options?: LDWaitForInitializationOptions,
@@ -65,14 +91,21 @@ const ldClientBridge = (namespace: string): LDClientBridge => ({
6591
stringVariationDetail: (key: string, defaultValue: string): LDEvaluationDetailTyped<string> =>
6692
ipcRenderer.sendSync(getIPCChannelName(namespace, 'stringVariationDetail'), key, defaultValue),
6793

68-
track: (key: string, data?: any, metricValue?: number): void =>
69-
ipcRenderer.sendSync(getIPCChannelName(namespace, 'track'), key, data, metricValue),
94+
track: (key: string, data?: any, metricValue?: number): void => {
95+
safeSendSync<void>(namespace, 'track', undefined, key, data, metricValue);
96+
},
7097

7198
variation: (key: string, defaultValue?: LDFlagValue): LDFlagValue =>
72-
ipcRenderer.sendSync(getIPCChannelName(namespace, 'variation'), key, defaultValue),
99+
safeSendSync<LDFlagValue>(namespace, 'variation', defaultValue, key, defaultValue),
73100

74101
variationDetail: (key: string, defaultValue?: LDFlagValue): LDEvaluationDetail =>
75-
ipcRenderer.sendSync(getIPCChannelName(namespace, 'variationDetail'), key, defaultValue),
102+
safeSendSync<LDEvaluationDetail>(
103+
namespace,
104+
'variationDetail',
105+
{ value: defaultValue, reason: { kind: 'ERROR', errorKind: 'EXCEPTION' } },
106+
key,
107+
defaultValue,
108+
),
76109

77110
setConnectionMode: (mode: ConnectionMode): Promise<void> =>
78111
ipcRenderer.invoke(getIPCChannelName(namespace, 'setConnectionMode'), mode),

0 commit comments

Comments
 (0)