Skip to content

Commit 9b2a216

Browse files
authored
fix: remove unnecessary precomputed constructor error logs (#229)
* Only check for salt and store initialization if the store is non-empty * One more test * Fix failing tests * Some eslint fixes * v4.12.1-alpha.0 * More lint warnings
1 parent d3f5d9e commit 9b2a216

File tree

3 files changed

+88
-27
lines changed

3 files changed

+88
-27
lines changed

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@eppo/js-client-sdk-common",
3-
"version": "4.12.0",
3+
"version": "4.12.1-alpha.0",
44
"description": "Common library for Eppo JavaScript SDKs (web, react native, and node)",
55
"main": "dist/index.js",
66
"files": [

src/client/eppo-precomputed-client.spec.ts

Lines changed: 72 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ describe('EppoPrecomputedClient E2E test', () => {
5252
subjectKey: 'test-subject',
5353
subjectAttributes: { attr1: 'value1' },
5454
};
55+
5556
beforeEach(async () => {
5657
storage = new MemoryOnlyConfigurationStore<PrecomputedFlag>();
5758
storage.setFormat(FormatEnum.PRECOMPUTED);
@@ -766,6 +767,76 @@ describe('EppoPrecomputedClient E2E test', () => {
766767
expect(loggedEvent.format).toEqual(FormatEnum.PRECOMPUTED);
767768
});
768769

770+
describe('Constructor logs errors according to the store state', () => {
771+
let mockError: jest.SpyInstance;
772+
773+
beforeEach(() => {
774+
mockError = jest.spyOn(logger, 'error');
775+
});
776+
777+
afterEach(() => {
778+
mockError.mockRestore();
779+
});
780+
781+
it('does not log errors when constructor receives an empty, uninitialized store', () => {
782+
const emptyStore = new MemoryOnlyConfigurationStore<PrecomputedFlag>();
783+
new EppoPrecomputedClient({
784+
precomputedFlagStore: emptyStore,
785+
subject: {
786+
subjectKey: '',
787+
subjectAttributes: {},
788+
},
789+
});
790+
expect(mockError).not.toHaveBeenCalledWith(
791+
'[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided',
792+
);
793+
expect(mockError).not.toHaveBeenCalledWith(
794+
'[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided',
795+
);
796+
});
797+
798+
it('logs errors when constructor receives an uninitialized store without a salt', () => {
799+
const nonemptyStore = new MemoryOnlyConfigurationStore<PrecomputedFlag>();
800+
// Incorrectly initialized: no salt, not set to initialized
801+
jest.spyOn(nonemptyStore, 'getKeys').mockReturnValue(['some-key']);
802+
803+
new EppoPrecomputedClient({
804+
precomputedFlagStore: nonemptyStore,
805+
subject: {
806+
subjectKey: '',
807+
subjectAttributes: {},
808+
},
809+
});
810+
expect(mockError).toHaveBeenCalledWith(
811+
'[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided',
812+
);
813+
expect(mockError).toHaveBeenCalledWith(
814+
'[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided',
815+
);
816+
});
817+
818+
it('only logs initialization error when constructor receives an uninitialized store with salt', () => {
819+
const nonemptyStore = new MemoryOnlyConfigurationStore<PrecomputedFlag>();
820+
nonemptyStore.salt = 'nacl';
821+
// Incorrectly initialized: no salt, not set to initialized
822+
jest.spyOn(nonemptyStore, 'getKeys').mockReturnValue(['some-key']);
823+
824+
new EppoPrecomputedClient({
825+
precomputedFlagStore: nonemptyStore,
826+
subject: {
827+
subjectKey: '',
828+
subjectAttributes: {},
829+
},
830+
});
831+
expect(mockError).toHaveBeenCalledWith(
832+
'[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided',
833+
);
834+
expect(mockError).not.toHaveBeenCalledWith(
835+
'[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided',
836+
);
837+
});
838+
});
839+
769840
describe('EppoPrecomputedClient subject data and store initialization', () => {
770841
let client: EppoPrecomputedClient;
771842
let store: IConfigurationStore<PrecomputedFlag>;
@@ -784,13 +855,7 @@ describe('EppoPrecomputedClient E2E test', () => {
784855
subject,
785856
});
786857
}).not.toThrow();
787-
expect(loggerErrorSpy).toHaveBeenCalledTimes(2);
788-
expect(loggerErrorSpy).toHaveBeenCalledWith(
789-
'[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided',
790-
);
791-
expect(loggerErrorSpy).toHaveBeenCalledWith(
792-
'[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided',
793-
);
858+
expect(loggerErrorSpy).toHaveBeenCalledTimes(0);
794859
loggerErrorSpy.mockRestore();
795860
expect(client.getStringAssignment('string-flag', 'default')).toBe('default');
796861
});
@@ -884,12 +949,6 @@ describe('Precomputed Bandit Store', () => {
884949
subject,
885950
});
886951

887-
expect(loggerErrorSpy).toHaveBeenCalledWith(
888-
'[Eppo SDK] EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided',
889-
);
890-
expect(loggerErrorSpy).toHaveBeenCalledWith(
891-
'[Eppo SDK] EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided',
892-
);
893952
expect(loggerErrorSpy).toHaveBeenCalledWith(
894953
'[Eppo SDK] Passing banditOptions without requestParameters requires an initialized precomputedBanditStore',
895954
);

src/client/eppo-precomputed-client.ts

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -100,13 +100,21 @@ export default class EppoPrecomputedClient {
100100
// Online-mode
101101
this.requestParameters = options.requestParameters;
102102
} else {
103-
// Offline-mode
104-
105-
// Offline mode depends on pre-populated IConfigurationStores (flags and bandits) to source configuration.
106-
if (!this.precomputedFlagStore.isInitialized()) {
107-
logger.error(
108-
`${loggerPrefix} EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided`,
109-
);
103+
// Offline-mode -- depends on pre-populated IConfigurationStores (flags and bandits) to source configuration.
104+
105+
// Allow an empty precomputedFlagStore to be passed in, but if it has items, ensure it was initialized properly.
106+
if (this.precomputedFlagStore.getKeys().length > 0) {
107+
if (!this.precomputedFlagStore.isInitialized()) {
108+
logger.error(
109+
`${loggerPrefix} EppoPrecomputedClient requires an initialized precomputedFlagStore if requestParameters are not provided`,
110+
);
111+
}
112+
113+
if (!this.precomputedFlagStore.salt) {
114+
logger.error(
115+
`${loggerPrefix} EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided`,
116+
);
117+
}
110118
}
111119

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

118-
if (!this.precomputedFlagStore.salt) {
119-
logger.error(
120-
`${loggerPrefix} EppoPrecomputedClient requires a precomputedFlagStore with a salt if requestParameters are not provided`,
121-
);
122-
}
123-
124126
if (this.precomputedBanditStore && !this.precomputedBanditStore.salt) {
125127
logger.warn(
126128
`${loggerPrefix} EppoPrecomputedClient missing or empty salt for precomputedBanditStore`,

0 commit comments

Comments
 (0)