Skip to content

Commit 42643d9

Browse files
authored
fix(electron-sdk): localstorage indexing (#1262)
This PR will fix the localstorage indexing by removing the previous nested structure: ``` ldcache-{credential} ``` To: ``` ldcache ``` The new structure is more consistent with other SDKs by calculating the storage index purely out of the context canonical keys. This PR will also serialize the write to this cache so that multiple instances of this SDK could exist. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes how all Electron SDK instances persist flag data by moving to a shared `ldcache` file and introducing singleton/queued flush behavior; this can affect cache isolation and persistence correctness across clients. Risk is mitigated by added tests, but storage/indexing regressions would impact offline/bootstrapped evaluations. > > **Overview** > Switches Electron SDK persistent flag/context caching from per-credential files (e.g. `ldcache-{credentialHash}`) to a single shared `ldcache` file via a singleton `ElectronStorage`, aligning cache indexing purely with context-derived keys. > > `ElectronStorage` now updates its in-memory cache first and **serializes disk flushes** through a queued atomic-write mechanism; initialization failures now surface as thrown errors (with cause) to the platform wrapper, which logs and swallows storage failures. > > Updates contract tests to reset the storage singleton between clients and removes the `singleton` capability flag, and adds coverage for `maxCachedContexts` eviction/disabled caching plus new `ElectronPlatform` error-logging tests. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 81826d7. 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/1262" 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 0e887ed commit 42643d9

9 files changed

Lines changed: 289 additions & 170 deletions

File tree

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

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,101 @@ describe('given an initialized ElectronClient with enableIPC: false and polling'
288288
});
289289
});
290290

291+
function makeMemoryStorage() {
292+
const data = new Map<string, string>();
293+
return {
294+
get: jest.fn(async (key: string) => data.get(key) ?? null),
295+
set: jest.fn(async (key: string, value: string) => {
296+
data.set(key, value);
297+
}),
298+
clear: jest.fn(async (key: string) => {
299+
data.delete(key);
300+
}),
301+
keys: () => Array.from(data.keys()),
302+
};
303+
}
304+
305+
describe('maxCachedContexts', () => {
306+
it('evicts the oldest cached context when the limit is exceeded', async () => {
307+
const storage = makeMemoryStorage();
308+
const mockedFetch = mockFetch(JSON.stringify(remoteFlagsMockData), 200);
309+
(ElectronPlatform as jest.Mock).mockReturnValue({
310+
crypto: new ElectronCrypto(),
311+
info: new ElectronInfo(),
312+
requests: {
313+
createEventSource: jest.fn(),
314+
fetch: mockedFetch,
315+
getEventSourceCapabilities: jest.fn(),
316+
},
317+
encoding: new ElectronEncoding(),
318+
storage,
319+
});
320+
321+
const client = createClient(clientSideId, DEFAULT_INITIAL_CONTEXT, {
322+
initialConnectionMode: 'polling',
323+
enableIPC: false,
324+
diagnosticOptOut: true,
325+
sendEvents: false,
326+
maxCachedContexts: 1,
327+
});
328+
await client.start();
329+
330+
// After start, context A's flags are cached.
331+
// Storage should contain: context index + context A data + context A freshness
332+
const keysAfterStart = storage.keys();
333+
const contextDataKeys = keysAfterStart.filter(
334+
(k) => !k.includes('ContextIndex') && !k.includes('_freshness'),
335+
);
336+
expect(contextDataKeys).toHaveLength(1);
337+
338+
// Identify a second context — context A should be evicted (maxCachedContexts: 1)
339+
await client.identify({ kind: 'user', key: 'context-b' });
340+
341+
const keysAfterIdentify = storage.keys();
342+
const contextDataKeysAfter = keysAfterIdentify.filter(
343+
(k) => !k.includes('ContextIndex') && !k.includes('_freshness'),
344+
);
345+
expect(contextDataKeysAfter).toHaveLength(1);
346+
347+
// The surviving key should be different from the first one (context B replaced context A)
348+
expect(contextDataKeysAfter[0]).not.toEqual(contextDataKeys[0]);
349+
});
350+
351+
it('does not cache flags when maxCachedContexts is 0', async () => {
352+
const storage = makeMemoryStorage();
353+
const mockedFetch = mockFetch(JSON.stringify(remoteFlagsMockData), 200);
354+
(ElectronPlatform as jest.Mock).mockReturnValue({
355+
crypto: new ElectronCrypto(),
356+
info: new ElectronInfo(),
357+
requests: {
358+
createEventSource: jest.fn(),
359+
fetch: mockedFetch,
360+
getEventSourceCapabilities: jest.fn(),
361+
},
362+
encoding: new ElectronEncoding(),
363+
storage,
364+
});
365+
366+
const client = createClient(clientSideId, DEFAULT_INITIAL_CONTEXT, {
367+
initialConnectionMode: 'polling',
368+
enableIPC: false,
369+
diagnosticOptOut: true,
370+
sendEvents: false,
371+
maxCachedContexts: 0,
372+
});
373+
await client.start();
374+
375+
// Flags should still evaluate correctly from the network response
376+
expect(client.boolVariation('on-off-flag', false)).toBe(true);
377+
378+
// But no context data should be persisted to storage
379+
const contextDataKeys = storage
380+
.keys()
381+
.filter((k) => !k.includes('ContextIndex') && !k.includes('_freshness'));
382+
expect(contextDataKeys).toHaveLength(0);
383+
});
384+
});
385+
291386
describe('given an initialized ElectronClient with enableIPC: false and streaming', () => {
292387
const logger: LDLogger = {
293388
debug: jest.fn(),
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
import type { LDLogger } from '@launchdarkly/js-client-sdk-common';
2+
3+
import ElectronPlatform from '../../src/platform/ElectronPlatform';
4+
5+
const failingStorage = {
6+
get: jest.fn().mockRejectedValue(new Error('disk read failed')),
7+
set: jest.fn().mockRejectedValue(new Error('disk write failed')),
8+
clear: jest.fn().mockRejectedValue(new Error('disk clear failed')),
9+
};
10+
11+
jest.mock('../../src/platform/ElectronStorage', () => ({
12+
getElectronStorage: () => failingStorage,
13+
}));
14+
15+
const logger: LDLogger = {
16+
debug: jest.fn(),
17+
info: jest.fn(),
18+
warn: jest.fn(),
19+
error: jest.fn(),
20+
};
21+
22+
let platform: ElectronPlatform;
23+
24+
beforeEach(() => {
25+
jest.clearAllMocks();
26+
platform = new ElectronPlatform(logger, {});
27+
});
28+
29+
it('logs error and returns null when storage get fails', async () => {
30+
const result = await platform.storage!.get('some-key');
31+
32+
expect(result).toBeNull();
33+
expect(logger.error).toHaveBeenCalledWith(
34+
expect.stringContaining('Error getting key from storage: some-key'),
35+
);
36+
});
37+
38+
it('logs error and swallows when storage set fails', async () => {
39+
await platform.storage!.set('some-key', 'some-value');
40+
41+
expect(logger.error).toHaveBeenCalledWith(
42+
expect.stringContaining('Error setting key in storage: some-key'),
43+
);
44+
});
45+
46+
it('logs error and swallows when storage clear fails', async () => {
47+
await platform.storage!.clear('some-key');
48+
49+
expect(logger.error).toHaveBeenCalledWith(
50+
expect.stringContaining('Error clearing key from storage: some-key'),
51+
);
52+
});

0 commit comments

Comments
 (0)