Bugfix for issue 121

splitio · Oct 15, 2024 · a1da884 · a1da884
1 parent ef9cf36
commit a1da884
Show file tree

Hide file tree

Showing 7 changed files with 65 additions and 49 deletions.
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -1,3 +1,6 @@
+1.14.1 (October 15, 2024)
+ - Bugfixing - Fixed error in `splitReducer` when handling actions with a `null` payload, preventing crashes caused by accessing undefined payload properties (Related to https://github.com/splitio/redux-client/issues/121).
+
 1.14.0 (September 13, 2024)
  - Added `status` property to Split reducer's slice of state to track the SDK events of non-default clients (Related to https://github.com/splitio/redux-client/issues/113).
  - Added `lastUpdate` and `isTimedout` properties to the object returned by the `getStatus` helper and `selectTreatmentAndStatus` and `selectTreatmentWithConfigAndStatus` selectors, to expose the last event timestamp and the timedout status of the SDK clients (Related to https://github.com/splitio/redux-client/issues/113).

diff --git a/src/__tests__/asyncActions.browser.test.ts b/src/__tests__/asyncActions.browser.test.ts
@@ -285,7 +285,8 @@ describe('getTreatments', () => {
           payload: {
             key: sdkBrowserConfig.core.key,
             treatments: expect.any(Object),
-            timestamp: expect.any(Number)
+            timestamp: expect.any(Number),
+            nonDefaultKey: false
           }
         });
 
@@ -364,7 +365,8 @@ describe('getTreatments', () => {
         payload: {
           key: sdkBrowserConfig.core.key,
           treatments: expect.any(Object),
-          timestamp: expect.any(Number)
+          timestamp: expect.any(Number),
+          nonDefaultKey: false
         }
       });
 
@@ -438,7 +440,8 @@ describe('getTreatments', () => {
         payload: {
           key: sdkBrowserConfig.core.key,
           treatments: expect.any(Object),
-          timestamp: expect.any(Number)
+          timestamp: expect.any(Number),
+          nonDefaultKey: false
         }
       });
 
@@ -459,7 +462,8 @@ describe('getTreatments', () => {
         payload: {
           key: sdkBrowserConfig.core.key,
           treatments: expect.any(Object),
-          timestamp: expect.any(Number)
+          timestamp: expect.any(Number),
+          nonDefaultKey: false
         }
       });
 
@@ -478,7 +482,8 @@ describe('getTreatments', () => {
         payload: {
           key: sdkBrowserConfig.core.key,
           treatments: expect.any(Object),
-          timestamp: expect.any(Number)
+          timestamp: expect.any(Number),
+          nonDefaultKey: false
         }
       });
 

diff --git a/src/__tests__/reducer.test.ts b/src/__tests__/reducer.test.ts
@@ -190,7 +190,7 @@ describe('Split reducer', () => {
     const initialTreatments = initialState.treatments;
 
     // default key
-    const action = actionCreator(key, treatments, 1000);
+    const action = actionCreator(key, treatments, 1000, false);
     expect(splitReducer(initialState, action)).toEqual({
       ...initialState,
       isReady,
@@ -231,7 +231,7 @@ describe('Split reducer', () => {
     const newTreatments: SplitIO.TreatmentsWithConfig = {
       test_split: { ...previousTreatment },
     };
-    const action = actionCreator(key, newTreatments, 1000);
+    const action = actionCreator(key, newTreatments, 1000, false);
     const reduxState = splitReducer(stateWithTreatments, action);
 
     // control assertion - treatment object was not replaced in the state
@@ -262,7 +262,7 @@ describe('Split reducer', () => {
         config: previousTreatment.config,
       },
     };
-    const action = actionCreator(key, newTreatments, 1000);
+    const action = actionCreator(key, newTreatments, 1000, false);
     const reduxState = splitReducer(stateWithTreatments, action);
 
     // control assertion - treatment object was replaced in the state
@@ -293,7 +293,7 @@ describe('Split reducer', () => {
       },
     };
     // const action = addTreatments(key, newTreatments);
-    const action = actionCreator(key, newTreatments, 1000);
+    const action = actionCreator(key, newTreatments, 1000, false);
     const reduxState = splitReducer(stateWithTreatments, action);
 
     // control assertion - treatment object was replaced in the state
@@ -314,4 +314,12 @@ describe('Split reducer', () => {
     });
   });
 
+  it('should ignore other actions', () => {
+    expect(splitReducer(initialState, { type: 'OTHER_ACTION' })).toBe(initialState);
+    expect(splitReducer(initialState, { type: 'OTHER_ACTION', payload: null })).toBe(initialState);
+    expect(splitReducer(initialState, { type: 'OTHER_ACTION', payload: undefined })).toBe(initialState);
+    expect(splitReducer(initialState, { type: 'OTHER_ACTION', payload: {} })).toBe(initialState);
+    expect(splitReducer(initialState, { type: 'OTHER_ACTION', payload: true })).toBe(initialState);
+  });
+
 });
diff --git a/src/actions.ts b/src/actions.ts
@@ -18,7 +18,7 @@ export function splitReady(timestamp: number, key?: SplitIO.SplitKey) {
   };
 }
 
-export function splitReadyWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey?: boolean) {
+export function splitReadyWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey: boolean) {
   return {
     type: SPLIT_READY_WITH_EVALUATIONS,
     payload: {
@@ -40,7 +40,7 @@ export function splitReadyFromCache(timestamp: number, key?: SplitIO.SplitKey) {
   };
 }
 
-export function splitReadyFromCacheWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey?: boolean) {
+export function splitReadyFromCacheWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey: boolean) {
   return {
     type: SPLIT_READY_FROM_CACHE_WITH_EVALUATIONS,
     payload: {
@@ -62,7 +62,7 @@ export function splitUpdate(timestamp: number, key?: SplitIO.SplitKey) {
   };
 }
 
-export function splitUpdateWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey?: boolean) {
+export function splitUpdateWithEvaluations(key: SplitIO.SplitKey, treatments: SplitIO.TreatmentsWithConfig, timestamp: number, nonDefaultKey: boolean) {
   return {
     type: SPLIT_UPDATE_WITH_EVALUATIONS,
     payload: {

diff --git a/src/asyncActions.ts b/src/asyncActions.ts
@@ -236,7 +236,7 @@ export function getClient(splitSdk: ISplitSdk, key?: SplitIO.SplitKey): IClientN
     if (client.evalOnReady.length) {
       const treatments = __getTreatments(client, client.evalOnReady);
 
-      splitSdk.dispatch(splitReadyWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key && true));
+      splitSdk.dispatch(splitReadyWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key ? true : false));
     } else {
       splitSdk.dispatch(splitReady(lastUpdate, key));
     }
@@ -255,7 +255,7 @@ export function getClient(splitSdk: ISplitSdk, key?: SplitIO.SplitKey): IClientN
     if (client.evalOnReadyFromCache.length) {
       const treatments = __getTreatments(client, client.evalOnReadyFromCache);
 
-      splitSdk.dispatch(splitReadyFromCacheWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key && true));
+      splitSdk.dispatch(splitReadyFromCacheWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key ? true : false));
     } else {
       splitSdk.dispatch(splitReadyFromCache(lastUpdate, key));
     }
@@ -270,7 +270,7 @@ export function getClient(splitSdk: ISplitSdk, key?: SplitIO.SplitKey): IClientN
     if (evalOnUpdate.length) {
       const treatments = __getTreatments(client, evalOnUpdate);
 
-      splitSdk.dispatch(splitUpdateWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key && true));
+      splitSdk.dispatch(splitUpdateWithEvaluations(key || (splitSdk.config as SplitIO.IBrowserSettings).core.key, treatments, lastUpdate, key ? true : false));
     } else {
       splitSdk.dispatch(splitUpdate(lastUpdate, key));
     }

diff --git a/src/reducer.ts b/src/reducer.ts
@@ -22,65 +22,66 @@ const initialState: ISplitState = {
   treatments: {},
 };
 
-function setStatus(state: ISplitState, patch: Partial<IStatus>, key?: string) {
-  return key ? {
+function setStatus(state: ISplitState, patch: Partial<IStatus>, action: ISplitAction) {
+  const { timestamp, key, nonDefaultKey } = action.payload;
+
+  return nonDefaultKey || (nonDefaultKey === undefined && key) ? {
     ...state,
     status: {
       ...state.status,
       [key]: state.status && state.status[key] ? {
         ...state.status[key],
         ...patch,
+        lastUpdate: timestamp,
       } : {
         ...initialStatus,
         ...patch,
+        lastUpdate: timestamp,
       }
     },
   } : {
     ...state,
     ...patch,
+    lastUpdate: timestamp,
   };
 }
 
-function setReady(state: ISplitState, timestamp: number, key?: string) {
+function setReady(state: ISplitState, action: ISplitAction) {
   return setStatus(state, {
     isReady: true,
     isTimedout: false,
-    lastUpdate: timestamp,
-  }, key);
+  }, action);
 }
 
-function setReadyFromCache(state: ISplitState, timestamp: number, key?: string) {
+function setReadyFromCache(state: ISplitState, action: ISplitAction) {
   return setStatus(state, {
     isReadyFromCache: true,
-    lastUpdate: timestamp,
-  }, key);
+  }, action);
 }
 
-function setTimedout(state: ISplitState, timestamp: number, key?: string) {
+function setTimedout(state: ISplitState, action: ISplitAction) {
   return setStatus(state, {
     isTimedout: true,
     hasTimedout: true,
-    lastUpdate: timestamp,
-  }, key);
+  }, action);
 }
 
-function setUpdated(state: ISplitState, timestamp: number, key?: string) {
-  return setStatus(state, {
-    lastUpdate: timestamp,
-  }, key);
+function setUpdated(state: ISplitState, action: ISplitAction) {
+  return setStatus(state, {}, action);
 }
 
-function setDestroyed(state: ISplitState, timestamp: number, key?: string) {
+function setDestroyed(state: ISplitState, action: ISplitAction) {
   return setStatus(state, {
     isDestroyed: true,
-    lastUpdate: timestamp,
-  }, key);
+  }, action);
 }
 
 /**
  * Copy the given `treatments` for the given `key` to a `result` Split's slice of state. Returns the `result` object.
  */
-function assignTreatments(result: ISplitState, key: string, treatments: SplitIO.TreatmentsWithConfig): ISplitState {
+function assignTreatments(result: ISplitState, action: ISplitAction): ISplitState {
+  const { key, treatments } = action.payload;
+
   result.treatments = { ...result.treatments };
   Object.entries<SplitIO.TreatmentWithConfig>(treatments).forEach(([featureFlagName, treatment]) => {
     if (result.treatments[featureFlagName]) {
@@ -106,42 +107,41 @@ export const splitReducer: Reducer<ISplitState> = function (
   state = initialState,
   action,
 ) {
-  const { type, payload: { timestamp, key, treatments, nonDefaultKey } = {} } = action as ISplitAction;
 
-  switch (type) {
+  switch (action.type) {
     case SPLIT_READY:
-      return setReady(state, timestamp, key);
+      return setReady(state, action as ISplitAction);
 
     case SPLIT_READY_FROM_CACHE:
-      return setReadyFromCache(state, timestamp, key);
+      return setReadyFromCache(state, action as ISplitAction);
 
     case SPLIT_TIMEDOUT:
-      return setTimedout(state, timestamp, key);
+      return setTimedout(state, action as ISplitAction);
 
     case SPLIT_UPDATE:
-      return setUpdated(state, timestamp, key);
+      return setUpdated(state, action as ISplitAction);
 
     case SPLIT_DESTROY:
-      return setDestroyed(state, timestamp, key);
+      return setDestroyed(state, action as ISplitAction);
 
     case ADD_TREATMENTS: {
       const result = { ...state };
-      return assignTreatments(result, key, treatments);
+      return assignTreatments(result, action as ISplitAction);
     }
 
     case SPLIT_READY_WITH_EVALUATIONS: {
-      const result = setReady(state, timestamp, nonDefaultKey && key);
-      return assignTreatments(result, key, treatments);
+      const result = setReady(state, action as ISplitAction);
+      return assignTreatments(result, action as ISplitAction);
     }
 
     case SPLIT_READY_FROM_CACHE_WITH_EVALUATIONS: {
-      const result = setReadyFromCache(state, timestamp, nonDefaultKey && key);
-      return assignTreatments(result, key, treatments);
+      const result = setReadyFromCache(state, action as ISplitAction);
+      return assignTreatments(result, action as ISplitAction);
     }
 
     case SPLIT_UPDATE_WITH_EVALUATIONS: {
-      const result = setUpdated(state, timestamp, nonDefaultKey && key);
-      return assignTreatments(result, key, treatments);
+      const result = setUpdated(state, action as ISplitAction);
+      return assignTreatments(result, action as ISplitAction);
     }
 
     default:

diff --git a/src/utils.ts b/src/utils.ts
@@ -6,7 +6,7 @@ import { IGetTreatmentsParams } from './types';
  * Validates if a given value is a plain object
  */
 export function isObject(obj: unknown) {
-  return obj && typeof obj === 'object' && obj.constructor === Object;
+  return obj !== null && typeof obj === 'object' && (obj.constructor === Object || (obj.constructor != null && obj.constructor.name === 'Object'));
 }
 
 /**