Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: remove unnecessary precomputed constructor error logs #229

Merged
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
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.12.0",
"version": "4.12.1-alpha.0",
"description": "Common library for Eppo JavaScript SDKs (web, react native, and node)",
"main": "dist/index.js",
"files": [
Expand Down
85 changes: 72 additions & 13 deletions src/client/eppo-precomputed-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ describe('EppoPrecomputedClient E2E test', () => {
subjectKey: 'test-subject',
subjectAttributes: { attr1: 'value1' },
};

beforeEach(async () => {
storage = new MemoryOnlyConfigurationStore<PrecomputedFlag>();
storage.setFormat(FormatEnum.PRECOMPUTED);
Expand Down Expand Up @@ -766,6 +767,76 @@ describe('EppoPrecomputedClient E2E test', () => {
expect(loggedEvent.format).toEqual(FormatEnum.PRECOMPUTED);
});

describe('Constructor logs errors according to the store state', () => {
let mockError: jest.SpyInstance;

beforeEach(() => {
mockError = jest.spyOn(logger, 'error');
});

afterEach(() => {
mockError.mockRestore();
});

it('does not log errors when constructor receives an empty, uninitialized store', () => {
const emptyStore = new MemoryOnlyConfigurationStore<PrecomputedFlag>();
new EppoPrecomputedClient({
precomputedFlagStore: emptyStore,
subject: {
subjectKey: '',
subjectAttributes: {},
},
});
expect(mockError).not.toHaveBeenCalledWith(
'[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided',
);
expect(mockError).not.toHaveBeenCalledWith(
'[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided',
);
});

it('logs errors when constructor receives an uninitialized store without a salt', () => {
const nonemptyStore = new MemoryOnlyConfigurationStore<PrecomputedFlag>();
// Incorrectly initialized: no salt, not set to initialized
jest.spyOn(nonemptyStore, 'getKeys').mockReturnValue(['some-key']);

new EppoPrecomputedClient({
precomputedFlagStore: nonemptyStore,
subject: {
subjectKey: '',
subjectAttributes: {},
},
});
expect(mockError).toHaveBeenCalledWith(
'[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided',
);
expect(mockError).toHaveBeenCalledWith(
'[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided',
);
});

it('only logs initialization error when constructor receives an uninitialized store with salt', () => {
const nonemptyStore = new MemoryOnlyConfigurationStore<PrecomputedFlag>();
nonemptyStore.salt = 'nacl';
// Incorrectly initialized: no salt, not set to initialized
jest.spyOn(nonemptyStore, 'getKeys').mockReturnValue(['some-key']);

new EppoPrecomputedClient({
precomputedFlagStore: nonemptyStore,
subject: {
subjectKey: '',
subjectAttributes: {},
},
});
expect(mockError).toHaveBeenCalledWith(
'[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided',
);
expect(mockError).not.toHaveBeenCalledWith(
'[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided',
);
});
});

describe('EppoPrecomputedClient subject data and store initialization', () => {
let client: EppoPrecomputedClient;
let store: IConfigurationStore<PrecomputedFlag>;
Expand All @@ -784,13 +855,7 @@ describe('EppoPrecomputedClient E2E test', () => {
subject,
});
}).not.toThrow();
expect(loggerErrorSpy).toHaveBeenCalledTimes(2);
expect(loggerErrorSpy).toHaveBeenCalledWith(
'[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided',
);
expect(loggerErrorSpy).toHaveBeenCalledWith(
'[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided',
);
expect(loggerErrorSpy).toHaveBeenCalledTimes(0);
loggerErrorSpy.mockRestore();
expect(client.getStringAssignment('string-flag', 'default')).toBe('default');
});
Expand Down Expand Up @@ -884,12 +949,6 @@ describe('Precomputed Bandit Store', () => {
subject,
});

expect(loggerErrorSpy).toHaveBeenCalledWith(
'[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided',
);
expect(loggerErrorSpy).toHaveBeenCalledWith(
'[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided',
);
expect(loggerErrorSpy).toHaveBeenCalledWith(
'[Eppo SDK] Passing banditOptions without requestParameters requires an initialized precomputedBanditStore',
);
Expand Down
28 changes: 15 additions & 13 deletions src/client/eppo-precomputed-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,13 +100,21 @@ export default class EppoPrecomputedClient {
// Online-mode
this.requestParameters = options.requestParameters;
} else {
// Offline-mode

// Offline mode depends on pre-populated IConfigurationStores (flags and bandits) to source configuration.
if (!this.precomputedFlagStore.isInitialized()) {
logger.error(
`${loggerPrefix} EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided`,
);
// Offline-mode -- depends on pre-populated IConfigurationStores (flags and bandits) to source configuration.

// Allow an empty precomputedFlagStore to be passed in, but if it has items, ensure it was initialized properly.
if (this.precomputedFlagStore.getKeys().length > 0) {
if (!this.precomputedFlagStore.isInitialized()) {
logger.error(
`${loggerPrefix} EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided`,
);
}

if (!this.precomputedFlagStore.salt) {
logger.error(
`${loggerPrefix} EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided`,
);
}
}

if (this.precomputedBanditStore && !this.precomputedBanditStore.isInitialized()) {
Expand All @@ -115,12 +123,6 @@ export default class EppoPrecomputedClient {
);
}

if (!this.precomputedFlagStore.salt) {
logger.error(
`${loggerPrefix} EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided`,
);
}

if (this.precomputedBanditStore && !this.precomputedBanditStore.salt) {
logger.warn(
`${loggerPrefix} EppoPrecomputedClient missing or empty salt for precomputedBanditStore`,
Expand Down
Loading