Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@eppo/js-client-sdk-common",
"version": "4.9.0",
"version": "4.9.1",
"description": "Common library for Eppo JavaScript SDKs (web, react native, and node)",
"main": "dist/index.js",
"files": [
Expand Down
68 changes: 40 additions & 28 deletions src/client/eppo-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import * as td from 'testdouble';

import {
ASSIGNMENT_TEST_DATA_DIR,
getTestAssignments,
IAssignmentTestCase,
MOCK_UFC_RESPONSE_FILE,
OBFUSCATED_MOCK_UFC_RESPONSE_FILE,
SubjectTestCase,
getTestAssignments,
readMockUFCResponse,
SubjectTestCase,
testCasesByFileName,
validateTestAssignments,
} from '../../test/testHelpers';
Expand All @@ -21,13 +21,13 @@ import {
} from '../configuration';
import { IConfigurationStore } from '../configuration-store/configuration-store';
import { MemoryOnlyConfigurationStore } from '../configuration-store/memory.store';
import { MAX_EVENT_QUEUE_SIZE, DEFAULT_POLL_INTERVAL_MS, POLL_JITTER_PCT } from '../constants';
import { DEFAULT_POLL_INTERVAL_MS, MAX_EVENT_QUEUE_SIZE, POLL_JITTER_PCT } from '../constants';
import { decodePrecomputedFlag } from '../decoding';
import { Flag, ObfuscatedFlag, VariationType } from '../interfaces';
import { Flag, FormatEnum, ObfuscatedFlag, VariationType } from '../interfaces';
import { getMD5Hash } from '../obfuscation';
import { AttributeType } from '../types';

import EppoClient, { FlagConfigurationRequestParameters, checkTypeMatch } from './eppo-client';
import EppoClient, { checkTypeMatch, FlagConfigurationRequestParameters } from './eppo-client';
import { initConfiguration } from './test-utils';

// Use a known salt to produce deterministic hashes
Expand All @@ -45,6 +45,18 @@ describe('EppoClient E2E test', () => {
}) as jest.Mock;
const storage = new MemoryOnlyConfigurationStore<Flag | ObfuscatedFlag>();

/**
* Use this helper instead of directly setting entries on the `storage` ConfigurationStore.
* This method ensures the format field is set as it is required for parsing.
* @param entries
*/
function setUnobfuscatedFlagEntries(
entries: Record<string, Flag | ObfuscatedFlag>,
): Promise<boolean> {
storage.setFormat(FormatEnum.SERVER);
return storage.setEntries(entries);
}

beforeAll(async () => {
await initConfiguration(storage);
});
Expand Down Expand Up @@ -87,8 +99,8 @@ describe('EppoClient E2E test', () => {
describe('error encountered', () => {
let client: EppoClient;

beforeAll(() => {
storage.setEntries({ [flagKey]: mockFlag });
beforeAll(async () => {
await setUnobfuscatedFlagEntries({ [flagKey]: mockFlag });
client = new EppoClient({ flagConfigurationStore: storage });

td.replace(EppoClient.prototype, 'getAssignmentDetail', function () {
Expand Down Expand Up @@ -143,8 +155,8 @@ describe('EppoClient E2E test', () => {
});

describe('setLogger', () => {
beforeAll(() => {
storage.setEntries({ [flagKey]: mockFlag });
beforeAll(async () => {
await setUnobfuscatedFlagEntries({ [flagKey]: mockFlag });
});

it('Invokes logger for queued events', () => {
Expand Down Expand Up @@ -191,8 +203,8 @@ describe('EppoClient E2E test', () => {
});

describe('precomputed flags', () => {
beforeAll(() => {
storage.setEntries({
beforeAll(async () => {
await setUnobfuscatedFlagEntries({
[flagKey]: mockFlag,
disabledFlag: { ...mockFlag, enabled: false },
anotherFlag: {
Expand Down Expand Up @@ -424,10 +436,10 @@ describe('EppoClient E2E test', () => {
);
});

it('logs variation assignment and experiment key', () => {
it('logs variation assignment and experiment key', async () => {
const mockLogger = td.object<IAssignmentLogger>();

storage.setEntries({ [flagKey]: mockFlag });
await setUnobfuscatedFlagEntries({ [flagKey]: mockFlag });
const client = new EppoClient({ flagConfigurationStore: storage });
client.setAssignmentLogger(mockLogger);

Expand All @@ -449,11 +461,11 @@ describe('EppoClient E2E test', () => {
expect(loggedAssignmentEvent.allocation).toEqual(mockFlag.allocations[0].key);
});

it('handles logging exception', () => {
it('handles logging exception', async () => {
const mockLogger = td.object<IAssignmentLogger>();
td.when(mockLogger.logAssignment(td.matchers.anything())).thenThrow(new Error('logging error'));

storage.setEntries({ [flagKey]: mockFlag });
await setUnobfuscatedFlagEntries({ [flagKey]: mockFlag });
const client = new EppoClient({ flagConfigurationStore: storage });
client.setAssignmentLogger(mockLogger);

Expand All @@ -468,8 +480,8 @@ describe('EppoClient E2E test', () => {
expect(assignment).toEqual('variation-a');
});

it('exports flag configuration', () => {
storage.setEntries({ [flagKey]: mockFlag });
it('exports flag configuration', async () => {
await setUnobfuscatedFlagEntries({ [flagKey]: mockFlag });
const client = new EppoClient({ flagConfigurationStore: storage });
expect(client.getFlagConfigurations()).toEqual({ [flagKey]: mockFlag });
});
Expand All @@ -478,10 +490,10 @@ describe('EppoClient E2E test', () => {
let client: EppoClient;
let mockLogger: IAssignmentLogger;

beforeEach(() => {
beforeEach(async () => {
mockLogger = td.object<IAssignmentLogger>();

storage.setEntries({ [flagKey]: mockFlag });
await setUnobfuscatedFlagEntries({ [flagKey]: mockFlag });
client = new EppoClient({ flagConfigurationStore: storage });
client.setAssignmentLogger(mockLogger);
});
Expand Down Expand Up @@ -536,7 +548,7 @@ describe('EppoClient E2E test', () => {
});

it('logs for each unique flag', async () => {
await storage.setEntries({
await setUnobfuscatedFlagEntries({
[flagKey]: mockFlag,
'flag-2': {
...mockFlag,
Expand All @@ -563,10 +575,10 @@ describe('EppoClient E2E test', () => {
expect(td.explain(mockLogger.logAssignment).callCount).toEqual(3);
});

it('logs twice for the same flag when allocations change', () => {
it('logs twice for the same flag when allocations change', async () => {
client.useNonExpiringInMemoryAssignmentCache();

storage.setEntries({
await setUnobfuscatedFlagEntries({
[flagKey]: {
...mockFlag,

Expand All @@ -587,7 +599,7 @@ describe('EppoClient E2E test', () => {
});
client.getStringAssignment(flagKey, 'subject-10', {}, 'default');

storage.setEntries({
await setUnobfuscatedFlagEntries({
[flagKey]: {
...mockFlag,
allocations: [
Expand All @@ -609,17 +621,17 @@ describe('EppoClient E2E test', () => {
expect(td.explain(mockLogger.logAssignment).callCount).toEqual(2);
});

it('logs the same subject/flag/variation after two changes', () => {
it('logs the same subject/flag/variation after two changes', async () => {
client.useNonExpiringInMemoryAssignmentCache();

// original configuration version
storage.setEntries({ [flagKey]: mockFlag });
await setUnobfuscatedFlagEntries({ [flagKey]: mockFlag });

client.getStringAssignment(flagKey, 'subject-10', {}, 'default'); // log this assignment
client.getStringAssignment(flagKey, 'subject-10', {}, 'default'); // cache hit, don't log

// change the variation
storage.setEntries({
await setUnobfuscatedFlagEntries({
[flagKey]: {
...mockFlag,
allocations: [
Expand All @@ -642,13 +654,13 @@ describe('EppoClient E2E test', () => {
client.getStringAssignment(flagKey, 'subject-10', {}, 'default'); // cache hit, don't log

// change the flag again, back to the original
storage.setEntries({ [flagKey]: mockFlag });
await setUnobfuscatedFlagEntries({ [flagKey]: mockFlag });

client.getStringAssignment(flagKey, 'subject-10', {}, 'default'); // important: log this assignment
client.getStringAssignment(flagKey, 'subject-10', {}, 'default'); // cache hit, don't log

// change the allocation
storage.setEntries({
await setUnobfuscatedFlagEntries({
[flagKey]: {
...mockFlag,
allocations: [
Expand Down
61 changes: 52 additions & 9 deletions src/client/eppo-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import { LRUInMemoryAssignmentCache } from '../cache/lru-in-memory-assignment-ca
import { NonExpiringInMemoryAssignmentCache } from '../cache/non-expiring-in-memory-cache-assignment';
import { TLRUInMemoryAssignmentCache } from '../cache/tlru-in-memory-assignment-cache';
import {
IConfigurationWire,
ConfigurationWireV1,
IConfigurationWire,
IPrecomputedConfiguration,
PrecomputedConfiguration,
} from '../configuration';
Expand All @@ -27,6 +27,7 @@ import {
DEFAULT_POLL_CONFIG_REQUEST_RETRIES,
DEFAULT_POLL_INTERVAL_MS,
DEFAULT_REQUEST_TIMEOUT_MS,
OBFUSCATED_FORMATS,
} from '../constants';
import { decodeFlag } from '../decoding';
import { EppoValue } from '../eppo_value';
Expand All @@ -46,6 +47,7 @@ import {
BanditVariation,
ConfigDetails,
Flag,
getFormatFromString,
IPrecomputedBandit,
ObfuscatedFlag,
PrecomputedFlag,
Expand Down Expand Up @@ -101,6 +103,12 @@ export type EppoClientParameters = {
banditVariationConfigurationStore?: IConfigurationStore<BanditVariation[]>;
banditModelConfigurationStore?: IConfigurationStore<BanditParameters>;
configurationRequestParameters?: FlagConfigurationRequestParameters;
/**
* Setting this value will have no side effects other than triggering a warning when the actual
* configuration's obfuscated does not match the value set here.
*
* @deprecated obfuscation is determined by inspecting the `format` field of the UFC response.
*/
isObfuscated?: boolean;
};

Expand All @@ -121,7 +129,9 @@ export default class EppoClient {
private assignmentCache?: AssignmentCache;
// whether to suppress any errors and return default values instead
private isGracefulFailureMode = true;
private isObfuscated: boolean;
private expectObfuscated: boolean;
private obfuscationMismatchWarningIssued = false;
private configObfuscatedCache?: boolean;
private requestPoller?: IPoller;
private readonly evaluator = new Evaluator();

Expand All @@ -138,7 +148,30 @@ export default class EppoClient {
this.banditVariationConfigurationStore = banditVariationConfigurationStore;
this.banditModelConfigurationStore = banditModelConfigurationStore;
this.configurationRequestParameters = configurationRequestParameters;
this.isObfuscated = isObfuscated;
this.expectObfuscated = isObfuscated;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 This looks like it sets expectObfuscation (to either true or false) even if user didn't pass any value, which may cause an unexpected warning

}

private maybeWarnAboutObfuscationMismatch(configObfuscated: boolean) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we develop elaborate warning for a deprecated flag?

  1. The warning mechanism is sufficiently complicated that there's a bug in it (see above comment).
  2. If we think this kind of warning is useful enough to justify mechanism, it makes sense to keep it and not deprecate the flag (or add expectObfuscation flag). (Although I don't think it's worth it.)
  3. If it's not that useful, I would rather remove it completely. Maybe issue a quick deprecation warning in constructor when isObfuscated is supplied

// Don't warn again if we did on the last check.
if (configObfuscated !== this.expectObfuscated && !this.obfuscationMismatchWarningIssued) {
this.obfuscationMismatchWarningIssued = true;
logger.warn(
`[Eppo SDK] configuration obfuscation [${configObfuscated}] does not match expected [${this.expectObfuscated}]`,
);
} else if (configObfuscated === this.expectObfuscated) {
// Reset the warning to false in case the client configuration (re-)enters a mismatched state.
this.obfuscationMismatchWarningIssued = false;
}
}

private isObfuscated() {
if (this.configObfuscatedCache === undefined) {
this.configObfuscatedCache = OBFUSCATED_FORMATS.includes(
getFormatFromString(this.flagConfigurationStore.getFormat()),
);
}
this.maybeWarnAboutObfuscationMismatch(this.configObfuscatedCache);
return this.configObfuscatedCache;
}

setConfigurationRequestParameters(
Expand All @@ -150,6 +183,7 @@ export default class EppoClient {
// noinspection JSUnusedGlobalSymbols
setFlagConfigurationStore(flagConfigurationStore: IConfigurationStore<Flag | ObfuscatedFlag>) {
this.flagConfigurationStore = flagConfigurationStore;
this.configObfuscatedCache = undefined;
}

// noinspection JSUnusedGlobalSymbols
Expand Down Expand Up @@ -188,8 +222,15 @@ export default class EppoClient {
}

// noinspection JSUnusedGlobalSymbols
/**
* Setting this value will have no side effects other than triggering a warning when the actual
* configuration's obfuscated does not match the value set here.
*
* @deprecated The client determines whether the configuration is obfuscated by inspection
* @param isObfuscated
*/
setIsObfuscated(isObfuscated: boolean) {
this.isObfuscated = isObfuscated;
this.expectObfuscated = isObfuscated;
}

async fetchFlagConfigurations() {
Expand Down Expand Up @@ -236,6 +277,7 @@ export default class EppoClient {

const pollingCallback = async () => {
if (await this.flagConfigurationStore.isExpired()) {
this.configObfuscatedCache = undefined;
return configurationRequestor.fetchAndStoreConfigurations();
}
};
Expand Down Expand Up @@ -854,7 +896,7 @@ export default class EppoClient {
configDetails,
subjectKey,
subjectAttributes,
this.isObfuscated,
this.isObfuscated(),
);

// allocationKey is set along with variation when there is a result. this check appeases typescript below
Expand Down Expand Up @@ -993,15 +1035,16 @@ export default class EppoClient {
);
}

const isObfuscated = this.isObfuscated();
const result = this.evaluator.evaluateFlag(
flag,
configDetails,
subjectKey,
subjectAttributes,
this.isObfuscated,
isObfuscated,
expectedVariationType,
);
if (this.isObfuscated) {
if (isObfuscated) {
// flag.key is obfuscated, replace with requested flag key
result.flagKey = flagKey;
}
Expand Down Expand Up @@ -1052,7 +1095,7 @@ export default class EppoClient {
}

private getFlag(flagKey: string): Flag | null {
return this.isObfuscated
return this.isObfuscated()
? this.getObfuscatedFlag(flagKey)
: this.flagConfigurationStore.get(flagKey);
}
Expand Down Expand Up @@ -1220,7 +1263,7 @@ export default class EppoClient {

private buildLoggerMetadata(): Record<string, unknown> {
return {
obfuscated: this.isObfuscated,
obfuscated: this.isObfuscated(),
sdkLanguage: 'javascript',
sdkLibVersion: LIB_VERSION,
};
Expand Down
7 changes: 7 additions & 0 deletions src/constants.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { FormatEnum } from './interfaces';

export const DEFAULT_REQUEST_TIMEOUT_MS = 5000;
export const REQUEST_TIMEOUT_MILLIS = DEFAULT_REQUEST_TIMEOUT_MS; // for backwards compatibility
export const DEFAULT_POLL_INTERVAL_MS = 30000;
Expand All @@ -15,3 +17,8 @@ export const NULL_SENTINEL = 'EPPO_NULL';
export const MAX_EVENT_QUEUE_SIZE = 100;
export const BANDIT_ASSIGNMENT_SHARDS = 10000;
export const DEFAULT_TLRU_TTL_MS = 600_000;

/**
* UFC Configuration formats which are obfuscated.
*/
export const OBFUSCATED_FORMATS = [FormatEnum.CLIENT, FormatEnum.PRECOMPUTED];
5 changes: 5 additions & 0 deletions src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,11 @@ export enum FormatEnum {
PRECOMPUTED = 'PRECOMPUTED',
}

export function getFormatFromString(str: string | null): FormatEnum {
// default to SERVER. Should always be set, but the ConfigurationStore allows null.
return FormatEnum[str as keyof typeof FormatEnum] ?? FormatEnum.SERVER;
}

export type BasePrecomputedFlag = {
flagKey?: string;
allocationKey?: string;
Expand Down
Loading