diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cd30e92..8c5648ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - fixed issue [#121](https://github.com/optimizely/react-sdk/issues/121):`ActivateListenerPayload` and `TrackListenerPayload` types were exported from `@optimizely/optimizely-sdk` but were missing from `@optimizely/react-sdk` exports. ([#150](https://github.com/optimizely/react-sdk/pull/150)). +### Bug fixes +- Fixed issue [#134](https://github.com/optimizely/react-sdk/issues/134) of the React SDK crashing when the internal Optimizely client returns as a null value. [PR #149](https://github.com/optimizely/react-sdk/pull/149) + ## [2.8.0] - January 26, 2022 ### New Features diff --git a/src/client.spec.ts b/src/client.spec.ts index 0d6473dd..0ea73cce 100644 --- a/src/client.spec.ts +++ b/src/client.spec.ts @@ -117,14 +117,17 @@ describe('ReactSDKClient', () => { }); describe('onReady', () => { + let instance: ReactSDKClient; + beforeEach(() => { + instance = createInstance(config); + }); + it('fulfills the returned promise with success: false when the timeout expires, and no user is set', async () => { - const instance = createInstance(config); const result = await instance.onReady({ timeout: 1 }); expect(result.success).toBe(false); }); it('fulfills the returned promise with success: true when a user is set', async () => { - const instance = createInstance(config); instance.setUser({ id: 'user12345', }); @@ -132,14 +135,50 @@ describe('ReactSDKClient', () => { expect(result.success).toBe(true); }); - it('waits for the inner client onReady to fulfill before fulfilling the returned promise', async () => { + describe('if Optimizely client is null', () => { + beforeEach(() => { + // Mocks dataReadyPromise value instead of _client = null because test initialization of instance causes dataReadyPromise to return { success: true } + // @ts-ignore + instance.dataReadyPromise = new Promise((resolve, reject) => { + resolve({ + success: false, + reason: 'NO_CLIENT', + message: 'Optimizely client failed to initialize.', + }); + }); + }); + + it('fulfills the returned promise with success: false when a user is set', async () => { + instance.setUser({ + id: 'user12345', + }); + const result = await instance.onReady(); + expect(result.success).toBe(false); + }); + + it('waits for the inner client onReady to fulfill with success = false before fulfilling the returned promise', async () => { + const mockInnerClientOnReady = jest.spyOn(mockInnerClient, 'onReady'); + let resolveInnerClientOnReady: (result: OnReadyResult) => void; + const mockReadyPromise: Promise = new Promise(res => { + resolveInnerClientOnReady = res; + }); + mockInnerClientOnReady.mockReturnValueOnce(mockReadyPromise); + instance.setUser({ + id: 'user999', + }); + resolveInnerClientOnReady!({ success: true }); + const result = await instance.onReady(); + expect(result.success).toBe(false); + }); + }); + + it('waits for the inner client onReady to fulfill with success = true before fulfilling the returned promise', async () => { const mockInnerClientOnReady = jest.spyOn(mockInnerClient, 'onReady'); let resolveInnerClientOnReady: (result: OnReadyResult) => void; const mockReadyPromise: Promise = new Promise(res => { resolveInnerClientOnReady = res; }); mockInnerClientOnReady.mockReturnValueOnce(mockReadyPromise); - const instance = createInstance(config); instance.setUser({ id: 'user999', }); @@ -199,6 +238,137 @@ describe('ReactSDKClient', () => { mockFn.mockReturnValue(['feat1', 'feat2']); }); + describe('if Optimizely client is null', () => { + beforeEach(() => { + // @ts-ignore + instance._client = null; + }); + + it('cannot use pre-set or override user for activate', () => { + const mockFn = mockInnerClient.activate as jest.Mock; + mockFn.mockReturnValue('var1'); + const result = instance.activate('exp1'); + expect(result).toBe(null); + }); + + it('cannot use pre-set or override user for track', () => { + const mockFn = mockInnerClient.track as jest.Mock; + instance.track('evt1'); + expect(mockFn).toBeCalledTimes(0); + }); + + it('cannot use pre-set or override user for isFeatureEnabled', () => { + const mockFn = mockInnerClient.isFeatureEnabled as jest.Mock; + mockFn.mockReturnValue(true); + const result = instance.isFeatureEnabled('feat1'); + expect(result).toBe(false); + }); + + it('cannot use pre-set and override user for getEnabledFeatures', () => { + const mockFn = mockInnerClient.getEnabledFeatures as jest.Mock; + mockFn.mockReturnValue(['feat1']); + const result = instance.getEnabledFeatures(); + expect(result).toEqual([]); + }); + + it('cannot use pre-set or override user for getVariation', () => { + const mockFn = mockInnerClient.getVariation as jest.Mock; + mockFn.mockReturnValue('var1'); + const result = instance.getVariation('exp1'); + expect(result).toEqual(null); + }); + + it('cannot use pre-set or override user for getFeatureVariableBoolean', () => { + const mockFn = mockInnerClient.getFeatureVariableBoolean as jest.Mock; + mockFn.mockReturnValue(false); + const result = instance.getFeatureVariableBoolean('feat1', 'bvar1'); + expect(result).toBe(null); + }); + + it('cannot use pre-set or override user for getFeatureVariableString', () => { + const mockFn = mockInnerClient.getFeatureVariableString as jest.Mock; + mockFn.mockReturnValue('varval1'); + const result = instance.getFeatureVariableString('feat1', 'svar1'); + expect(result).toBe(null); + }); + + it('cannot use pre-set or override user for getFeatureVariableInteger', () => { + const mockFn = mockInnerClient.getFeatureVariableInteger as jest.Mock; + mockFn.mockReturnValue(15); + const result = instance.getFeatureVariableInteger('feat1', 'ivar1'); + expect(result).toBe(null); + }); + + it('cannot use pre-set or override user for getFeatureVariableDouble', () => { + const mockFn = mockInnerClient.getFeatureVariableDouble as jest.Mock; + mockFn.mockReturnValue(15.5); + const result = instance.getFeatureVariableDouble('feat1', 'dvar1'); + expect(result).toBe(null); + }); + + it('cannot use pre-set or override user for getFeatureVariableJSON', () => { + const mockFn = mockInnerClient.getFeatureVariableJSON as jest.Mock; + mockFn.mockReturnValue({ + num_buttons: 0, + text: 'default value', + }); + const result = instance.getFeatureVariableJSON('feat1', 'dvar1'); + expect(result).toEqual(null); + }); + + it('cannot use pre-set or override user for getFeatureVariable', () => { + const mockFn = mockInnerClient.getFeatureVariable as jest.Mock; + mockFn.mockReturnValue({ + num_buttons: 0, + text: 'default value', + }); + const result = instance.getFeatureVariable('feat1', 'dvar1', 'user1'); + expect(result).toEqual(null); + }); + + it('cannot use pre-set or override user for setForcedVariation', () => { + const mockFn = mockInnerClient.setForcedVariation as jest.Mock; + mockFn.mockReturnValue(true); + const result = instance.setForcedVariation('exp1', 'var1'); + expect(result).toBe(false); + }); + + it('cannot use pre-set or override user for getForcedVariation', () => { + const mockFn = mockInnerClient.getForcedVariation as jest.Mock; + mockFn.mockReturnValue('var1'); + const result = instance.getForcedVariation('exp1'); + expect(result).toBe(null); + }); + + it('cannot use pre-set or override user for decide', () => { + const mockFn = mockOptimizelyUserContext.decide as jest.Mock; + mockFn.mockReturnValue({ + enabled: true, + flagKey: 'theFlag1', + reasons: [], + ruleKey: '', + userContext: mockOptimizelyUserContext, + variables: {}, + variationKey: 'varition1', + }); + const result = instance.decide('exp1'); + expect(result).toEqual({ + enabled: false, + flagKey: 'exp1', + reasons: ['Unable to evaluate flag exp1 because Optimizely client failed to initialize.'], + ruleKey: null, + userContext: { + attributes: { + foo: 'bar', + }, + id: 'user1', + }, + variables: {}, + variationKey: null, + }); + }); + }); + it('can use pre-set and override user for activate', () => { const mockFn = mockInnerClient.activate as jest.Mock; mockFn.mockReturnValue('var1'); @@ -529,6 +699,28 @@ describe('ReactSDKClient', () => { expect(mockCreateUserContext).toBeCalledWith('user2', { bar: 'baz' }); }); + describe('if Optimizely client is null', () => { + it('cannot use pre-set or override user for decideAll', () => { + const mockFn = mockOptimizelyUserContext.decideAll as jest.Mock; + const mockCreateUserContext = mockInnerClient.createUserContext as jest.Mock; + mockFn.mockReturnValue({ + theFlag1: { + enabled: true, + flagKey: 'theFlag1', + reasons: [], + ruleKey: '', + userContext: mockOptimizelyUserContext, + variables: {}, + variationKey: 'varition1', + }, + }); + // @ts-ignore + instance._client = null; + const result = instance.decideAll(); + expect(result).toEqual({}); + }); + }); + it('can use pre-set and override user for decideAll', () => { const mockFn = mockOptimizelyUserContext.decideAll as jest.Mock; const mockCreateUserContext = mockInnerClient.createUserContext as jest.Mock; @@ -593,6 +785,28 @@ describe('ReactSDKClient', () => { expect(mockCreateUserContext).toBeCalledWith('user2', { bar: 'baz' }); }); + describe('if Optimizely client is null', () => { + it('cannot use pre-set or override user for decideForKeys', () => { + const mockFn = mockOptimizelyUserContext.decideForKeys as jest.Mock; + const mockCreateUserContext = mockInnerClient.createUserContext as jest.Mock; + mockFn.mockReturnValue({ + theFlag1: { + enabled: true, + flagKey: 'theFlag1', + reasons: [], + ruleKey: '', + userContext: mockOptimizelyUserContext, + variables: {}, + variationKey: 'varition1', + }, + }); + // @ts-ignore + instance._client = null; + const result = instance.decideForKeys(['theFlag1']); + expect(result).toEqual({}); + }); + }); + it('can use pre-set and override user for decideForKeys', () => { const mockFn = mockOptimizelyUserContext.decideForKeys as jest.Mock; const mockCreateUserContext = mockInnerClient.createUserContext as jest.Mock; @@ -668,6 +882,75 @@ describe('ReactSDKClient', () => { expect(result).toEqual({}); }); + describe('if Optimizely client is null', () => { + it('does not return an object with variables of all types returned from the inner sdk ', () => { + (mockInnerClient.getOptimizelyConfig as jest.Mock).mockReturnValue({ + featuresMap: { + feat1: { + variablesMap: { + bvar: { + id: '0', + key: 'bvar', + type: 'boolean', + value: 'false', + }, + svar: { + id: '1', + key: 'svar', + type: 'string', + value: '', + }, + ivar: { + id: '2', + key: 'ivar', + type: 'integer', + value: '0', + }, + dvar: { + id: '3', + key: 'dvar', + type: 'double', + value: '0', + }, + jvar: { + id: '4', + key: 'jvar', + type: 'json', + value: '{}', + }, + }, + }, + }, + }); + (mockInnerClient.getFeatureVariable as jest.Mock).mockImplementation( + (featureKey: string, variableKey: string) => { + switch (variableKey) { + case 'bvar': + return true; + case 'svar': + return 'whatsup'; + case 'ivar': + return 10; + case 'dvar': + return -10.5; + case 'jvar': + return { value: 'json value' }; + default: + return null; + } + } + ); + const instance = createInstance(config); + instance.setUser({ + id: 'user1123', + }); + // @ts-ignore + instance._client = null; + const result = instance.getFeatureVariables('feat1'); + expect(result).toEqual({}); + }); + }); + it('returns an object with variables of all types returned from the inner sdk ', () => { (mockInnerClient.getOptimizelyConfig as jest.Mock).mockReturnValue({ featuresMap: { @@ -743,6 +1026,53 @@ describe('ReactSDKClient', () => { }); describe('getAllFeatureVariables', () => { + describe('if Optimizely client is null', () => { + it('does not return an object with variables of all types returned from the inner sdk ', () => { + const anyClient = mockInnerClient.getAllFeatureVariables as jest.Mock; + anyClient.mockReturnValue({ + bvar: true, + svar: 'whatsup', + ivar: 10, + dvar: -10.5, + jvar: { + value: 'json value', + }, + }); + const instance = createInstance(config); + // @ts-ignore + instance._client = null; + instance.setUser({ + id: 'user1123', + }); + const result = instance.getAllFeatureVariables('feat1', 'user1'); + expect(result).toEqual({}); + }); + + it('cannot use pre-set and override user for getAllFeatureVariables', () => { + const mockFn = mockInnerClient.getAllFeatureVariables as jest.Mock; + mockFn.mockReturnValue({ + bvar: true, + svar: 'whatsup', + ivar: 10, + dvar: -10.5, + jvar: { + value: 'json value', + }, + }); + const instance = createInstance(config); + // @ts-ignore + instance._client = null; + instance.setUser({ + id: 'user1', + attributes: { + foo: 'bar', + }, + }); + const result = instance.getAllFeatureVariables('feat1', 'user1'); + expect(result).toEqual({}); + }); + }); + it('returns an empty object when the inner SDK returns no variables', () => { const anyClient = mockInnerClient.getAllFeatureVariables as jest.Mock; anyClient.mockReturnValue({}); @@ -850,6 +1180,17 @@ describe('ReactSDKClient', () => { }); }); + describe('if Optimizely client is null', () => { + it('does not call the handler function when setForcedVariation is called', () => { + // @ts-ignore + instance._client = null; + const handler = jest.fn(); + instance.onForcedVariationsUpdate(handler); + instance.setForcedVariation('my_exp', 'xxfueaojfe8&86', 'variation_a'); + expect(handler).toBeCalledTimes(0); + }); + }); + it('calls the handler function when setForcedVariation is called', () => { const handler = jest.fn(); instance.onForcedVariationsUpdate(handler); @@ -882,6 +1223,24 @@ describe('ReactSDKClient', () => { expect(result).toEqual(false); }); + describe('if Optimizely client is null', () => { + it('should return false', () => { + // @ts-ignore + instance._client = null; + instance.setUser({ + id: 'user1', + }); + const mockFn = mockOptimizelyUserContext.removeAllForcedDecisions as jest.Mock; + + mockFn.mockReturnValue(true); + + const result = instance.removeAllForcedDecisions(); + expect(mockFn).toBeCalledTimes(0); + expect(result).toBeDefined(); + expect(result).toEqual(false); + }); + }); + it('should return true if user context has been set ', () => { instance.setUser({ id: 'user1', @@ -909,6 +1268,38 @@ describe('ReactSDKClient', () => { }); }); + describe('when Optimizely client is null', () => { + it('should report an error', () => { + const mockFn = mockOptimizelyUserContext.decide as jest.Mock; + mockFn.mockReturnValue({ + enabled: true, + flagKey: 'theFlag1', + reasons: [], + ruleKey: '', + userContext: mockOptimizelyUserContext, + variables: {}, + variationKey: 'varition1', + }); + + // @ts-ignore + instance._client = null; + + const result = instance.decide('theFlag1'); + expect(result).toEqual({ + enabled: false, + flagKey: 'theFlag1', + reasons: ['Unable to evaluate flag theFlag1 because Optimizely client failed to initialize.'], + ruleKey: null, + userContext: { + id: 'user1', + attributes: { foo: 'bar' }, + }, + variables: {}, + variationKey: null, + }); + }); + }); + it('should overwrite decide when forcedDecision is envoked', () => { const mockFn = mockOptimizelyUserContext.decide as jest.Mock; mockFn.mockReturnValue({ @@ -986,6 +1377,38 @@ describe('ReactSDKClient', () => { }); }); + describe('when Optimizely client is null', () => { + it('should report flag evaluation error', () => { + const mockFn = mockOptimizelyUserContext.decide as jest.Mock; + mockFn.mockReturnValue({ + enabled: true, + flagKey: 'theFlag1', + reasons: [], + ruleKey: '', + userContext: mockOptimizelyUserContext, + variables: {}, + variationKey: 'varition1', + }); + + // @ts-ignore + instance._client = null; + + const result = instance.decide('theFlag1'); + expect(result).toEqual({ + enabled: false, + flagKey: 'theFlag1', + reasons: ['Unable to evaluate flag theFlag1 because Optimizely client failed to initialize.'], + ruleKey: null, + userContext: { + id: 'user1', + attributes: { foo: 'bar' }, + }, + variables: {}, + variationKey: null, + }); + }); + }); + it('should revert back to the decide default value when removeForcedDecision is envoked after settingup the forced decision', () => { const mockFn = mockOptimizelyUserContext.decide as jest.Mock; mockFn.mockReturnValue({ diff --git a/src/client.ts b/src/client.ts index 5faf1791..86eb2db9 100644 --- a/src/client.ts +++ b/src/client.ts @@ -32,9 +32,12 @@ type OnUserUpdateHandler = (userInfo: UserInfo) => void; type OnForcedVariationsUpdateHandler = () => void; +type NotReadyReason = 'TIMEOUT' | 'NO_CLIENT'; + export type OnReadyResult = { success: boolean; - reason?: string; + reason?: NotReadyReason; + message?: string; dataReadyPromise?: Promise; }; @@ -189,18 +192,18 @@ class OptimizelyReactSDKClient implements ReactSDKClient { private forcedDecisionFlagKeys: Set = new Set(); // Is the javascript SDK instance ready. - private isClientReady: boolean = false; + private isClientReady = false; // We need to add autoupdate listener to the hooks after the instance became fully ready to avoid redundant updates to hooks - private isReadyPromiseFulfilled: boolean = false; + private isReadyPromiseFulfilled = false; // Its usually true from the beginning when user is provided as an object in the `OptimizelyProvider` // This becomes more significant when a promise is provided instead. - private isUserReady: boolean = false; + private isUserReady = false; - private isUsingSdkKey: boolean = false; + private isUsingSdkKey = false; - private readonly _client: optimizely.Client; + private readonly _client: optimizely.Client | null; // promise keeping track of async requests for initializing client instance private dataReadyPromise: Promise; @@ -212,15 +215,15 @@ class OptimizelyReactSDKClient implements ReactSDKClient { constructor(config: optimizely.Config) { this.initialConfig = config; - this.userPromiseResolver = () => {}; + this.userPromiseResolver = () => { }; const configWithClientInfo = { ...config, clientEngine: REACT_SDK_CLIENT_ENGINE, clientVersion: REACT_SDK_CLIENT_VERSION, }; - this._client = optimizely.createInstance(configWithClientInfo); + this._client = optimizely.createInstance(configWithClientInfo); this.isClientReady = !!configWithClientInfo.datafile; this.isUsingSdkKey = !!configWithClientInfo.sdkKey; @@ -231,18 +234,30 @@ class OptimizelyReactSDKClient implements ReactSDKClient { return { success: true }; }); - this._client.onReady().then(() => { - this.isClientReady = true; - }); - - this.dataReadyPromise = Promise.all([this.userPromise, this._client.onReady()]).then(() => { - // Client and user can become ready synchronously and/or asynchronously. This flag specifically indicates that they became ready asynchronously. - this.isReadyPromiseFulfilled = true; - return { - success: true, - reason: 'datafile and user resolved', - }; - }); + if (this._client) { + this._client.onReady().then(() => { + this.isClientReady = true; + }); + + this.dataReadyPromise = Promise.all([this.userPromise, this._client!.onReady()]).then(() => { + // Client and user can become ready synchronously and/or asynchronously. This flag specifically indicates that they became ready asynchronously. + this.isReadyPromiseFulfilled = true; + return { + success: true, + message: 'Successfully resolved datafile and user information.', + }; + }); + } else { + logger.warn('Unable to resolve datafile and user information because Optimizely client failed to initialize.'); + + this.dataReadyPromise = new Promise((resolve, reject) => { + resolve({ + success: false, + reason: 'NO_CLIENT', + message: 'Optimizely client failed to initialize.', + }); + }); + } } getIsReadyPromiseFulfilled(): boolean { @@ -264,7 +279,8 @@ class OptimizelyReactSDKClient implements ReactSDKClient { timeoutId = setTimeout(() => { resolve({ success: false, - reason: + reason: 'TIMEOUT', + message: 'failed to initialize onReady before timeout, either the datafile or user info was not set before the timeout', dataReadyPromise: this.dataReadyPromise, }); @@ -278,6 +294,14 @@ class OptimizelyReactSDKClient implements ReactSDKClient { } getUserContextInstance(userInfo: UserInfo): optimizely.OptimizelyUserContext | null { + if (!this._client) { + logger.warn( + 'Unable to get user context for user id "%s" because Optimizely client failed to initialize.', + userInfo.id + ); + return null; + } + let userContext: optimizely.OptimizelyUserContext | null = null; if (this.userContext) { @@ -304,11 +328,17 @@ class OptimizelyReactSDKClient implements ReactSDKClient { setUser(userInfo: UserInfo): void { // TODO add check for valid user if (userInfo.id) { - const userContext = this._client.createUserContext(userInfo.id, userInfo.attributes); - this.user.id = userInfo.id; this.isUserReady = true; - this.userContext = userContext; + + if (this._client) { + this.userContext = this._client.createUserContext(userInfo.id, userInfo.attributes); + } else { + logger.warn( + 'Unable to create user context for user id "%s" because Optimizely client failed to initialize.', + this.user.id + ); + } } if (userInfo.attributes) { @@ -369,11 +399,18 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideUserId?: string, overrideAttributes?: optimizely.UserAttributes ): string | null { + if (!this._client) { + logger.warn('Unable to activate experiment "%s" because Optimizely client failed to initialize.', experimentKey); + return null; + } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); + if (user.id === null) { - logger.info('Not activating experiment "%s" because userId is not set', experimentKey); + logger.info('Unable to activate experiment "%s" because User ID is not set', experimentKey); return null; } + return this._client.activate(experimentKey, user.id, user.attributes); } @@ -383,10 +420,20 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideUserId?: string, overrideAttributes?: optimizely.UserAttributes ): OptimizelyDecision { + if (!this._client) { + logger.warn('Unable to evaluate feature "%s" because Optimizely client failed to initialize.', key); + return createFailedDecision( + key, + `Unable to evaluate flag ${key} because Optimizely client failed to initialize.`, + this.user + ); + } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); + if (user.id === null) { - logger.info('Not Evaluating feature "%s" because userId is not set', key); - return createFailedDecision(key, `Not Evaluating flag ${key} because userId is not set`, user); + logger.info('Unable to evaluate feature "%s" because User ID is not set.', key); + return createFailedDecision(key, `Unable to evaluate flag ${key} because User ID is not set.`, user); } const optlyUserContext = this.getUserContextInstance(user); @@ -408,9 +455,15 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideUserId?: string, overrideAttributes?: optimizely.UserAttributes ): { [key: string]: OptimizelyDecision } { + if (!this._client) { + logger.warn('Unable to evaluate features for keys because Optimizely client failed to initialize.'); + return {}; + } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); + if (user.id === null) { - logger.info('Not Evaluating features because userId is not set'); + logger.info('Unable to evaluate features for keys because User ID is not set'); return {}; } @@ -438,9 +491,15 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideUserId?: string, overrideAttributes?: optimizely.UserAttributes ): { [key: string]: OptimizelyDecision } { + if (!this._client) { + logger.warn('Unable to evaluate all feature decisions because Optimizely client is not initialized.'); + return {}; + } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); + if (user.id === null) { - logger.info('Not Evaluating features because userId is not set'); + logger.info('Unable to evaluate all feature decisions because User ID is not set'); return {}; } @@ -476,11 +535,21 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideUserId?: string, overrideAttributes?: optimizely.UserAttributes ): string | null { + if (!this._client) { + logger.warn( + 'Unable to get variation for experiment "%s" because Optimizely client failed to initialize.', + experimentKey + ); + return null; + } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); + if (user.id === null) { - logger.info('getVariation returned null for experiment "%s" because userId is not set', experimentKey); + logger.info('Unable to get variation for experiment "%s" because User ID is not set', experimentKey); return null; } + return this._client.getVariation(experimentKey, user.id, user.attributes); } @@ -498,19 +567,25 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideAttributes?: optimizely.UserAttributes, eventTags?: optimizely.EventTags ): void { + if (!this._client) { + logger.warn('Unable to send tracking event "%s" because Optimizely client failed to initialize.', eventKey); + return; + } + if (typeof overrideUserId !== 'undefined' && typeof overrideUserId !== 'string') { eventTags = overrideUserId; overrideUserId = undefined; overrideAttributes = undefined; } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); if (user.id === null) { - logger.info('track for event "%s" not being sent because userId is not set', eventKey); + logger.info('Unable to send tracking event "%s" because User ID is not set', eventKey); return; } - return this._client.track(eventKey, user.id, user.attributes, eventTags); + this._client.track(eventKey, user.id, user.attributes, eventTags); } /** @@ -524,7 +599,7 @@ class OptimizelyReactSDKClient implements ReactSDKClient { decision: optimizely.OptimizelyForcedDecision ): void { if (!this.userContext) { - logger.info("Can't set a forced decision because the user context has not been set yet"); + logger.warn('Unable to set a forced decision because the user context has not been set yet.'); return; } @@ -546,7 +621,7 @@ class OptimizelyReactSDKClient implements ReactSDKClient { decisionContext: optimizely.OptimizelyDecisionContext ): optimizely.OptimizelyForcedDecision | null { if (!this.userContext) { - logger.info("Can't get a forced decision because the user context has not been set yet"); + logger.warn('Unable to get a forced decision because the user context has not been set yet.'); return null; } return this.userContext.getForcedDecision(decisionContext); @@ -560,7 +635,7 @@ class OptimizelyReactSDKClient implements ReactSDKClient { */ public removeForcedDecision(decisionContext: optimizely.OptimizelyDecisionContext): boolean { if (!this.userContext) { - logger.info("Can't remove forced decisions because the user context has not been set yet"); + logger.warn('Unable to remove forced decisions because the user context has not been set yet.'); return false; } @@ -581,7 +656,7 @@ class OptimizelyReactSDKClient implements ReactSDKClient { */ public removeAllForcedDecisions(): boolean { if (!this.userContext) { - logger.info("Can't remove a forced decision because the user context has not been set yet"); + logger.warn('Unable to remove all forced decisions because the user context has not been set yet.'); return false; } @@ -608,11 +683,21 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideUserId?: string, overrideAttributes?: optimizely.UserAttributes ): boolean { + if (!this._client) { + logger.warn( + 'Unable to determine if feature "%s" is enabled because Optimizely client failed to initialize.', + feature + ); + return false; + } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); + if (user.id === null) { - logger.info('isFeatureEnabled returning false for feature "%s" because userId is not set', feature); + logger.info('Unable to determine if feature "%s" is enabled because User ID is not set', feature); return false; } + return this._client.isFeatureEnabled(feature, user.id, user.attributes); } @@ -635,25 +720,46 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideUserId?: string, overrideAttributes?: optimizely.UserAttributes ): VariableValuesObject { + if (!this._client) { + logger.warn( + 'Unable to get feature variables for feature "%s" because Optimizely client failed to initialize.', + featureKey + ); + return {}; + } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); + const userId = user.id; if (userId === null) { - logger.info('getFeatureVariables returning `{}` for feature "%s" because userId is not set', featureKey); + logger.warn('Unable to get feature variables for feature "%s" because User ID is not set', featureKey); return {}; } + const userAttributes = user.attributes; const variableObj: VariableValuesObject = {}; + const optlyConfig = this._client.getOptimizelyConfig(); if (!optlyConfig) { + logger.warn( + 'Unable to retrieve feature variables for feature "%s" because Optimizely client failed to initialize.', + featureKey + ); return {}; } + const feature = optlyConfig.featuresMap[featureKey]; if (!feature) { + logger.info( + 'Unable to retrieve feature variables for feature "%s" because config features map is not set', + featureKey + ); return {}; } + Object.keys(feature.variablesMap).forEach(key => { const variable = feature.variablesMap[key]; - variableObj[variable.key] = this._client.getFeatureVariable(featureKey, variable.key, userId, userAttributes); + variableObj[variable.key] = this._client!.getFeatureVariable(featureKey, variable.key, userId, userAttributes); }); return variableObj; @@ -675,10 +781,21 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideUserId?: string, overrideAttributes?: optimizely.UserAttributes ): string | null { + if (!this._client) { + logger.warn( + 'Unable to get feature variable string from feature "%s" because Optimizely client failed to initialize.', + feature + ); + return null; + } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); + if (user.id === null) { + logger.info('Unable to get feature variable string from feature "%s" because User ID is not set', feature); return null; } + return this._client.getFeatureVariableString(feature, variable, user.id, user.attributes); } @@ -698,10 +815,21 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideUserId?: string, overrideAttributes?: optimizely.UserAttributes ): boolean | null { + if (!this._client) { + logger.warn( + 'Unable to get feature variable boolean from feature "%s" because Optimizely client failed to initialize.', + feature + ); + return null; + } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); + if (user.id === null) { + logger.info('Unable to get feature variable boolean from feature "%s" because User ID is not set', feature); return null; } + return this._client.getFeatureVariableBoolean(feature, variable, user.id, user.attributes); } @@ -721,10 +849,21 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideUserId?: string, overrideAttributes?: optimizely.UserAttributes ): number | null { + if (!this._client) { + logger.warn( + 'Unable to get feature variable integer from feature "%s" because Optimizely client failed to initialize.', + feature + ); + return null; + } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); + if (user.id === null) { + logger.info('Unable to get feature variable integer from feature "%s" because User ID is not set', feature); return null; } + return this._client.getFeatureVariableInteger(feature, variable, user.id, user.attributes); } @@ -744,10 +883,21 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideUserId?: string, overrideAttributes?: optimizely.UserAttributes ): number | null { + if (!this._client) { + logger.warn( + 'Unable to get feature variable double from feature "%s" because Optimizely client failed to initialize.', + feature + ); + return null; + } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); + if (user.id === null) { + logger.info('Unable to get feature variable double from feature "%s" because User ID is not set', feature); return null; } + return this._client.getFeatureVariableDouble(feature, variable, user.id, user.attributes); } @@ -767,10 +917,21 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideUserId?: string, overrideAttributes?: optimizely.UserAttributes ): unknown { + if (!this._client) { + logger.warn( + 'Unable to get feature variable JSON from feature "%s" because Optimizely client failed to initialize.', + feature + ); + return null; + } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); + if (user.id === null) { + logger.info('Unable to get feature variable JSON from feature "%s" because User ID is not set', feature); return null; } + return this._client.getFeatureVariableJSON(feature, variable, user.id, user.attributes); } @@ -790,10 +951,26 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideUserId: string, overrideAttributes?: optimizely.UserAttributes ): unknown { + if (!this._client) { + logger.warn( + 'Unable to get feature variable from feature "%s" because Optimizely client failed to initialize.', + featureKey, + variableKey + ); + return null; + } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); + if (user.id === null) { + logger.info( + 'Unable to get feature variable from feature "%s" because User ID is not set', + featureKey, + variableKey + ); return null; } + return this._client.getFeatureVariable(featureKey, variableKey, user.id, user.attributes); } @@ -810,10 +987,21 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideUserId: string, overrideAttributes?: optimizely.UserAttributes ): { [variableKey: string]: unknown } | null { + if (!this._client) { + logger.warn( + 'Unable to get all feature variables from feature "%s" because Optimizely client failed to initialize.', + featureKey + ); + return {}; + } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); + if (user.id === null) { + logger.info('Unable to get all feature variables from feature "%s" because User ID is not set', featureKey); return {}; } + return this._client.getAllFeatureVariables(featureKey, user.id, user.attributes); } @@ -825,10 +1013,18 @@ class OptimizelyReactSDKClient implements ReactSDKClient { * @memberof OptimizelyReactSDKClient */ public getEnabledFeatures(overrideUserId?: string, overrideAttributes?: optimizely.UserAttributes): Array { + if (!this._client) { + logger.warn('Unable to get list of enabled features because Optimizely client failed to initialize.'); + return []; + } + const user = this.getUserContextWithOverrides(overrideUserId, overrideAttributes); + if (user.id === null) { + logger.info('Unable to get list of enabled features because User ID is not set'); return []; } + return this._client.getEnabledFeatures(user.id, user.attributes); } @@ -840,10 +1036,21 @@ class OptimizelyReactSDKClient implements ReactSDKClient { * @memberof OptimizelyReactSDKClient */ public getForcedVariation(experiment: string, overrideUserId?: string): string | null { + if (!this._client) { + logger.warn( + 'Unable to get forced variation for experiment "%s" because Optimizely client failed to initialize.', + experiment + ); + return null; + } + const user = this.getUserContextWithOverrides(overrideUserId); + if (user.id === null) { + logger.info('Unable to get forced variation for experiment "%s" because User ID is not set', experiment); return null; } + return this._client.getForcedVariation(experiment, user.id); } @@ -860,6 +1067,14 @@ class OptimizelyReactSDKClient implements ReactSDKClient { overrideUserIdOrVariationKey: string, variationKey?: string | null ): boolean { + if (!this._client) { + logger.warn( + 'Unable to set forced variation for experiment "%s" because Optimizely client failed to initialize.', + experiment + ); + return false; + } + let finalUserId: string | null = null; let finalVariationKey: string | null = null; if (arguments.length === 2) { @@ -875,8 +1090,10 @@ class OptimizelyReactSDKClient implements ReactSDKClient { } if (finalUserId === null) { + logger.warn('Unable to set forced variation for experiment "%s" because User ID is not set', experiment); return false; } + const result = this._client.setForcedVariation(experiment, finalUserId, finalVariationKey); this.onForcedVariationsUpdateHandlers.forEach(handler => handler()); return result; @@ -887,24 +1104,66 @@ class OptimizelyReactSDKClient implements ReactSDKClient { * @returns {optimizely.OptimizelyConfig | null} optimizely config */ public getOptimizelyConfig(): optimizely.OptimizelyConfig | null { + if (!this._client) { + logger.warn('Unable to get Optimizely configuration because Optimizely client failed to initialize.'); + return null; + } return this._client.getOptimizelyConfig(); } /** * Cleanup method for killing an running timers and flushing eventQueue + * @returns {Promise<{ success: boolean; reason?: string }>} */ - public close() { + public close(): Promise<{ success: boolean; reason?: string }> { + if (!this._client) { + /** + * Note: + * - Returns a promise that resolves as "true" even when this._client does not exist. + * - This is because cleanup relies on close() resolving as "true". + * - If we resolve as "false", then the cleanup for timers and the event queue will never trigger. + * - Not triggering cleanup may lead to memory leaks and other inefficiencies. + */ + return new Promise<{ success: boolean; reason: string }>((resolve, reject) => + resolve({ + success: true, + reason: 'Optimizely client is not initialized.', + }) + ); + } return this._client.close(); } /** * Provide access to inner optimizely.Client object */ - public get client(): optimizely.Client { + public get client(): optimizely.Client | null { + if (!this._client) { + logger.warn('Unable to get client because Optimizely client failed to initialize.'); + return null; + } return this._client; } public get notificationCenter(): optimizely.NotificationCenter { + if (!this._client) { + return { + addNotificationListener: () => { + logger.warn('Unable to add notification listener because Optimizely client failed to initialize.'); + return 0; + }, + removeNotificationListener: () => { + logger.warn('Unable to remove notification listener because Optimizely client failed to initialize.'); + return false; + }, + clearAllNotificationListeners: () => { + logger.warn('Unable to clear all notification listeners because Optimizely client failed to initialize.'); + }, + clearNotificationListeners: () => { + logger.warn('Unable to clear notification listeners because Optimizely client failed to initialize.'); + }, + }; + } return this._client.notificationCenter; } diff --git a/src/hooks.spec.tsx b/src/hooks.spec.tsx index 8bcde621..ce226282 100644 --- a/src/hooks.spec.tsx +++ b/src/hooks.spec.tsx @@ -77,7 +77,7 @@ describe('hooks', () => { beforeEach(() => { getOnReadyPromise = ({ timeout = 0 }: any): Promise => new Promise(resolve => { - setTimeout(function() { + setTimeout(function () { resolve( Object.assign( { @@ -109,13 +109,13 @@ describe('hooks', () => { isFeatureEnabled: isFeatureEnabledMock, onUserUpdate: jest.fn().mockImplementation(handler => { userUpdateCallbacks.push(handler); - return () => {}; + return () => { }; }), notificationCenter: { addNotificationListener: jest.fn().mockImplementation((type, handler) => { notificationListenerCallbacks.push(handler); }), - removeNotificationListener: jest.fn().mockImplementation(id => {}), + removeNotificationListener: jest.fn().mockImplementation(id => { }), }, user: { id: 'testuser', @@ -126,7 +126,7 @@ describe('hooks', () => { getIsUsingSdkKey: () => true, onForcedVariationsUpdate: jest.fn().mockImplementation(handler => { forcedVariationUpdateCallbacks.push(handler); - return () => {}; + return () => { }; }), getForcedVariations: jest.fn().mockReturnValue({}), decide: decideMock, diff --git a/src/hooks.ts b/src/hooks.ts index a6dec605..eab40fa2 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -135,18 +135,39 @@ function subscribeToInitialization( }); return; } - hooksLogger.info(`Client did not become ready before timeout of ${timeout}ms, reason="${res.reason || ''}"`); - onInitStateChange({ - clientReady: false, - didTimeout: true, - }); - res.dataReadyPromise!.then(() => { - hooksLogger.info('Client became ready after timeout already elapsed'); - onInitStateChange({ - clientReady: true, - didTimeout: true, - }); - }); + + switch (res.reason) { + // Optimizely client failed to initialize. + case 'NO_CLIENT': + hooksLogger.warn(`Client not ready, reason="${res.message}"`); + onInitStateChange({ + clientReady: false, + didTimeout: false, + }); + res.dataReadyPromise!.then(() => { + hooksLogger.info('Client became ready.'); + onInitStateChange({ + clientReady: true, + didTimeout: false, + }); + }); + break; + // Assume timeout for all other cases. + // TODO: Other reasons may fall into this case - need to update later to specify 'TIMEOUT' case and general fallback case. + default: + hooksLogger.info(`Client did not become ready before timeout of ${timeout}ms, reason="${res.message}"`); + onInitStateChange({ + clientReady: false, + didTimeout: true, + }); + res.dataReadyPromise!.then(() => { + hooksLogger.info('Client became ready after timeout already elapsed'); + onInitStateChange({ + clientReady: true, + didTimeout: true, + }); + }); + } }) .catch(() => { hooksLogger.error(`Error initializing client. The core client or user promise(s) rejected.`); @@ -236,7 +257,7 @@ export const useExperiment: UseExperiment = (experimentKey, options = {}, overri })); }); } - return (): void => {}; + return (): void => { }; }, [optimizely.getIsReadyPromiseFulfilled(), options.autoUpdate, optimizely, experimentKey, getCurrentDecision]); useEffect( @@ -329,7 +350,7 @@ export const useFeature: UseFeature = (featureKey, options = {}, overrides = {}) })); }); } - return (): void => {}; + return (): void => { }; }, [optimizely.getIsReadyPromiseFulfilled(), options.autoUpdate, optimizely, featureKey, getCurrentDecision]); return [state.isEnabled, state.variables, state.clientReady, state.didTimeout]; @@ -358,11 +379,11 @@ export const useDecision: UseDecision = (flagKey, options = {}, overrides = {}) const decisionState = isClientReady ? getCurrentDecision() : { - decision: createFailedDecision(flagKey, 'Optimizely SDK not configured properly yet.', { - id: overrides.overrideUserId || null, - attributes: overrideAttrs, - }), - }; + decision: createFailedDecision(flagKey, 'Optimizely SDK not configured properly yet.', { + id: overrides.overrideUserId || null, + attributes: overrideAttrs, + }), + }; return { ...decisionState, clientReady: isClientReady, @@ -428,7 +449,7 @@ export const useDecision: UseDecision = (flagKey, options = {}, overrides = {}) })); }); } - return (): void => {}; + return (): void => { }; }, [optimizely.getIsReadyPromiseFulfilled(), options.autoUpdate, optimizely, flagKey, getCurrentDecision]); return [state.decision, state.clientReady, state.didTimeout];