Skip to content

Commit c9dd157

Browse files
felipecslsameerank
andauthored
fix(events): Allow disabling event logging, check r/o FS (#96)
* fix(events): Allow disabling event logging, check r/o FS * pass in config into newDefaultEventDispatcher * Update src/index.spec.ts Co-authored-by: Sameeran Kunche <[email protected]> * sort lines alphabetically --------- Co-authored-by: Sameeran Kunche <[email protected]>
1 parent ead7c25 commit c9dd157

File tree

4 files changed

+222
-10
lines changed

4 files changed

+222
-10
lines changed

src/i-client-config.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,26 @@ export interface IClientConfig {
4545

4646
/** Amount of time in milliseconds to wait between API calls to refresh configuration data. Default of 30_000 (30s). */
4747
pollingIntervalMs?: number;
48+
49+
/** Configuration settings for the event dispatcher */
50+
eventIngestionConfig?: {
51+
/** Maximum number of events to send per delivery request. Defaults to 1000 events. */
52+
batchSize?: number;
53+
/** Number of milliseconds to wait between each batch delivery. Defaults to 10 seconds. */
54+
deliveryIntervalMs?: number;
55+
/** Whether to disable event ingestion. Defaults to false. */
56+
disabled?: boolean;
57+
/**
58+
* Maximum number of events to queue in memory before starting to drop events.
59+
* Note: This is only used if localStorage is not available.
60+
* Defaults to 10000 events.
61+
*/
62+
maxQueueSize?: number;
63+
/** Maximum number of retry attempts before giving up on a batch delivery. Defaults to 3 retries. */
64+
maxRetries?: number;
65+
/** Maximum amount of milliseconds to wait before retrying a failed delivery. Defaults to 30 seconds. */
66+
maxRetryDelayMs?: number;
67+
/** Minimum amount of milliseconds to wait before retrying a failed delivery. Defaults to 5 seconds */
68+
retryIntervalMs?: number;
69+
};
4870
}

src/index.spec.ts

Lines changed: 127 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ import {
3030
validateTestAssignments,
3131
} from '../test/testHelpers';
3232

33-
import { getInstance, IAssignmentEvent, IAssignmentLogger, init } from '.';
33+
import * as util from './util/index';
34+
35+
import { getInstance, IAssignmentEvent, IAssignmentLogger, init, NO_OP_EVENT_DISPATCHER } from '.';
3436

3537
import SpyInstance = jest.SpyInstance;
3638

@@ -367,22 +369,34 @@ describe('EppoClient E2E test', () => {
367369
describe('Bandit assignment cache', () => {
368370
const flagKey = 'banner_bandit_flag'; // piggyback off a shared test data flag
369371
const bobKey = 'bob';
370-
const bobAttributes: Attributes = { age: 25, country: 'USA', gender_identity: 'female' };
372+
const bobAttributes: Attributes = {
373+
age: 25,
374+
country: 'USA',
375+
gender_identity: 'female',
376+
};
371377
const bobActions: Record<string, Attributes> = {
372378
nike: { brand_affinity: 1.5, loyalty_tier: 'silver' },
373379
adidas: { brand_affinity: -1.0, loyalty_tier: 'bronze' },
374380
reebok: { brand_affinity: 0.5, loyalty_tier: 'gold' },
375381
};
376382

377383
const aliceKey = 'alice';
378-
const aliceAttributes: Attributes = { age: 25, country: 'USA', gender_identity: 'female' };
384+
const aliceAttributes: Attributes = {
385+
age: 25,
386+
country: 'USA',
387+
gender_identity: 'female',
388+
};
379389
const aliceActions: Record<string, Attributes> = {
380390
nike: { brand_affinity: 1.5, loyalty_tier: 'silver' },
381391
adidas: { brand_affinity: -1.0, loyalty_tier: 'bronze' },
382392
reebok: { brand_affinity: 0.5, loyalty_tier: 'gold' },
383393
};
384394
const charlieKey = 'charlie';
385-
const charlieAttributes: Attributes = { age: 25, country: 'USA', gender_identity: 'female' };
395+
const charlieAttributes: Attributes = {
396+
age: 25,
397+
country: 'USA',
398+
gender_identity: 'female',
399+
};
386400
const charlieActions: Record<string, Attributes> = {
387401
nike: { brand_affinity: 1.0, loyalty_tier: 'gold' },
388402
adidas: { brand_affinity: 1.0, loyalty_tier: 'silver' },
@@ -452,7 +466,11 @@ describe('EppoClient E2E test', () => {
452466

453467
describe('Best Bandit Action', () => {
454468
const flagKey = 'banner_bandit_flag'; // piggyback off a shared test data flag
455-
const bobAttributes: Attributes = { age: 25, country: 'USA', gender_identity: 'female' };
469+
const bobAttributes: Attributes = {
470+
age: 25,
471+
country: 'USA',
472+
gender_identity: 'female',
473+
};
456474
const bobActions: Record<string, Attributes> = {
457475
nike: { brand_affinity: -10.5, loyalty_tier: 'silver' },
458476
adidas: { brand_affinity: 1.0, loyalty_tier: 'bronze' },
@@ -469,7 +487,6 @@ describe('EppoClient E2E test', () => {
469487

470488
it('Should return the highest ranked bandit', async () => {
471489
const client = getInstance();
472-
473490
const bestAction = client.getBestAction(flagKey, bobAttributes, bobActions, 'default');
474491

475492
expect(bestAction).toEqual('adidas');
@@ -590,4 +607,108 @@ describe('EppoClient E2E test', () => {
590607
expect(client.getStringAssignment(flagKey, 'subject', {}, 'default-value')).toBe('control');
591608
});
592609
});
610+
611+
describe('eventIngestionConfig', () => {
612+
it('should not be used if eventIngestionConfig.disabled is true', async () => {
613+
const client = await init({
614+
apiKey,
615+
baseUrl: `http://127.0.0.1:${TEST_SERVER_PORT}`,
616+
assignmentLogger: mockLogger,
617+
eventIngestionConfig: {
618+
disabled: true,
619+
},
620+
});
621+
expect(client['eventDispatcher']).toEqual(NO_OP_EVENT_DISPATCHER);
622+
});
623+
624+
it('should be used if eventIngestionConfig.disabled is false', async () => {
625+
const client = await init({
626+
apiKey,
627+
baseUrl: `http://127.0.0.1:${TEST_SERVER_PORT}`,
628+
assignmentLogger: mockLogger,
629+
eventIngestionConfig: {
630+
disabled: false,
631+
},
632+
});
633+
expect(client['eventDispatcher']).not.toEqual(NO_OP_EVENT_DISPATCHER);
634+
});
635+
636+
describe('read-only file system handling', () => {
637+
// Save original implementation
638+
let isReadOnlyFsSpy: SpyInstance;
639+
640+
beforeEach(() => {
641+
// Reset the module before each test
642+
jest.resetModules();
643+
// Create a spy on isReadOnlyFs that we can mock
644+
isReadOnlyFsSpy = jest.spyOn(util, 'isReadOnlyFs');
645+
});
646+
647+
afterEach(() => {
648+
isReadOnlyFsSpy.mockRestore();
649+
});
650+
651+
it('should use BoundedEventQueue when file system is read-only', async () => {
652+
// Mock isReadOnlyFs to return true (read-only file system)
653+
isReadOnlyFsSpy.mockReturnValue(true);
654+
const client = await init({
655+
apiKey,
656+
baseUrl: `http://127.0.0.1:${TEST_SERVER_PORT}`,
657+
assignmentLogger: mockLogger,
658+
eventIngestionConfig: {
659+
disabled: false,
660+
},
661+
});
662+
663+
// Check that the event queue is a BoundedEventQueue
664+
// We can't directly check the type, but we can check that it's not a FileBackedNamedEventQueue
665+
// by checking if the queue has a 'queueDirectory' property
666+
const eventDispatcher = client['eventDispatcher'];
667+
const eventQueue = eventDispatcher['batchProcessor']['eventQueue'];
668+
expect(eventQueue).toBeDefined();
669+
expect(eventQueue['queueDirectory']).toBeUndefined();
670+
});
671+
672+
it('should use FileBackedNamedEventQueue when file system is writable', async () => {
673+
// Mock isReadOnlyFs to return false (writable file system)
674+
isReadOnlyFsSpy.mockReturnValue(false);
675+
const client = await init({
676+
apiKey,
677+
baseUrl: `http://127.0.0.1:${TEST_SERVER_PORT}`,
678+
assignmentLogger: mockLogger,
679+
eventIngestionConfig: {
680+
disabled: false,
681+
},
682+
});
683+
684+
// Check that the event queue is a FileBackedNamedEventQueue
685+
// by checking if the queue has a 'queueDirectory' property
686+
const eventDispatcher = client['eventDispatcher'];
687+
const eventQueue = eventDispatcher['batchProcessor']['eventQueue'];
688+
expect(eventQueue).toBeDefined();
689+
expect(eventQueue['queueDirectory']).toBeDefined();
690+
});
691+
692+
it('should use BoundedEventQueue when isReadOnlyFs throws an error', async () => {
693+
// Mock isReadOnlyFs to throw an error
694+
isReadOnlyFsSpy.mockImplementation(() => {
695+
throw new Error('Test error');
696+
});
697+
const client = await init({
698+
apiKey,
699+
baseUrl: `http://127.0.0.1:${TEST_SERVER_PORT}`,
700+
assignmentLogger: mockLogger,
701+
eventIngestionConfig: {
702+
disabled: false,
703+
},
704+
});
705+
706+
// Check that the event queue is a BoundedEventQueue
707+
const eventDispatcher = client['eventDispatcher'];
708+
const eventQueue = eventDispatcher['batchProcessor']['eventQueue'];
709+
expect(eventQueue).toBeDefined();
710+
expect(eventQueue['queueDirectory']).toBeUndefined();
711+
});
712+
});
713+
});
593714
});

src/index.ts

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,25 @@ import {
33
BanditActions,
44
BanditParameters,
55
BanditVariation,
6+
BoundedEventQueue,
67
ContextAttributes,
78
EppoClient,
89
Event,
10+
EventDispatcher,
911
Flag,
1012
FlagConfigurationRequestParameters,
1113
FlagKey,
1214
MemoryOnlyConfigurationStore,
15+
NamedEventQueue,
16+
applicationLogger,
1317
newDefaultEventDispatcher,
1418
} from '@eppo/js-client-sdk-common';
1519

1620
import FileBackedNamedEventQueue from './events/file-backed-named-event-queue';
1721
import { IClientConfig } from './i-client-config';
1822
import { sdkName, sdkVersion } from './sdk-data';
1923
import { generateSalt } from './util';
24+
import { isReadOnlyFs } from './util/index';
2025

2126
export {
2227
Attributes,
@@ -35,6 +40,13 @@ export { IClientConfig };
3540

3641
let clientInstance: EppoClient;
3742

43+
export const NO_OP_EVENT_DISPATCHER: EventDispatcher = {
44+
// eslint-disable-next-line @typescript-eslint/no-empty-function
45+
attachContext: () => {},
46+
// eslint-disable-next-line @typescript-eslint/no-empty-function
47+
dispatch: () => {},
48+
};
49+
3850
/**
3951
* Initializes the Eppo client with configuration parameters.
4052
* This method should be called once on application startup.
@@ -52,6 +64,7 @@ export async function init(config: IClientConfig): Promise<EppoClient> {
5264
pollingIntervalMs,
5365
throwOnFailedInitialization = true,
5466
pollAfterFailedInitialization = false,
67+
eventIngestionConfig,
5568
} = config;
5669
if (!apiKey) {
5770
throw new Error('API key is required');
@@ -75,7 +88,7 @@ export async function init(config: IClientConfig): Promise<EppoClient> {
7588
const flagConfigurationStore = new MemoryOnlyConfigurationStore<Flag>();
7689
const banditVariationConfigurationStore = new MemoryOnlyConfigurationStore<BanditVariation[]>();
7790
const banditModelConfigurationStore = new MemoryOnlyConfigurationStore<BanditParameters>();
78-
const eventDispatcher = newEventDispatcher(apiKey);
91+
const eventDispatcher = newEventDispatcher(apiKey, eventIngestionConfig);
7992

8093
clientInstance = new EppoClient({
8194
flagConfigurationStore,
@@ -130,10 +143,48 @@ export function getInstance(): EppoClient {
130143
return clientInstance;
131144
}
132145

133-
function newEventDispatcher(sdkKey: string) {
134-
const eventQueue = new FileBackedNamedEventQueue<Event>('events');
146+
function newEventDispatcher(
147+
sdkKey: string,
148+
config: IClientConfig['eventIngestionConfig'] = {},
149+
): EventDispatcher {
150+
const {
151+
batchSize = 1_000,
152+
deliveryIntervalMs = 10_000,
153+
disabled = false,
154+
maxQueueSize = 10_000,
155+
maxRetries = 3,
156+
maxRetryDelayMs = 30_000,
157+
retryIntervalMs = 5_000,
158+
} = config;
159+
if (disabled) {
160+
return NO_OP_EVENT_DISPATCHER;
161+
}
162+
let eventQueue: NamedEventQueue<Event>;
163+
try {
164+
// Check if the file system is read-only
165+
if (isReadOnlyFs(`${process.cwd()}/.queues`)) {
166+
applicationLogger.warn(
167+
'File system appears to be read-only. Using in-memory event queue instead.',
168+
);
169+
eventQueue = new BoundedEventQueue<Event>('events', [], maxQueueSize);
170+
} else {
171+
eventQueue = new FileBackedNamedEventQueue<Event>('events');
172+
}
173+
} catch (error) {
174+
// If there's any error during the check, fall back to BoundedEventQueue
175+
applicationLogger.warn(
176+
`Error checking file system: ${error}. Using in-memory event queue instead.`,
177+
);
178+
eventQueue = new BoundedEventQueue<Event>('events', [], maxQueueSize);
179+
}
180+
135181
const emptyNetworkStatusListener =
136182
// eslint-disable-next-line @typescript-eslint/no-empty-function
137183
{ isOffline: () => false, onNetworkStatusChange: () => {} };
138-
return newDefaultEventDispatcher(eventQueue, emptyNetworkStatusListener, sdkKey);
184+
return newDefaultEventDispatcher(eventQueue, emptyNetworkStatusListener, sdkKey, batchSize, {
185+
deliveryIntervalMs,
186+
retryIntervalMs,
187+
maxRetryDelayMs,
188+
maxRetries,
189+
});
139190
}

src/util/index.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import * as fs from 'fs';
2+
import * as path from 'path';
3+
4+
/**
5+
* Checks if the file system is read-only by attempting to write to a temporary file.
6+
* @param directory The directory to check for write access
7+
* @returns True if the file system is read-only, false otherwise
8+
*/
9+
export function isReadOnlyFs(directory: string): boolean {
10+
try {
11+
const testFilePath = path.join(directory, '.write_test_' + Date.now());
12+
fs.writeFileSync(testFilePath, 'test', { flag: 'w' });
13+
fs.unlinkSync(testFilePath);
14+
return false;
15+
} catch (error) {
16+
return true;
17+
}
18+
}

0 commit comments

Comments
 (0)