fix: Hooks gracefully return with an error message when optimizely prop is null (#154)

Author: opti-jnguyen

CHANGELOG.md

@@ -12,6 +12,8 @@ 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)).
 
+- addresses issues [#152](https://github.com/optimizely/react-sdk/issues/152) and [#134](https://github.com/optimizely/react-sdk/issues/134): Gracefully returns pessimistic default values when hooks fail instead of throwing an error.
+
 ### 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)
 

src/Experiment.spec.tsx

@@ -45,10 +45,10 @@ describe('<OptimizelyExperiment>', () => {
     optimizelyMock = ({
       onReady: jest.fn().mockImplementation(config => onReadyPromise),
       activate: jest.fn().mockImplementation(experimentKey => variationKey),
-      onUserUpdate: jest.fn().mockImplementation(handler => () => {}),
+      onUserUpdate: jest.fn().mockImplementation(handler => () => { }),
       notificationCenter: {
-        addNotificationListener: jest.fn().mockImplementation((type, handler) => {}),
-        removeNotificationListener: jest.fn().mockImplementation(id => {}),
+        addNotificationListener: jest.fn().mockImplementation((type, handler) => { }),
+        removeNotificationListener: jest.fn().mockImplementation(id => { }),
       },
       user: {
         id: 'testuser',
@@ -57,15 +57,15 @@ describe('<OptimizelyExperiment>', () => {
       isReady: jest.fn().mockImplementation(() => isReady),
       getIsReadyPromiseFulfilled: () => true,
       getIsUsingSdkKey: () => true,
-      onForcedVariationsUpdate: jest.fn().mockReturnValue(() => {}),
+      onForcedVariationsUpdate: jest.fn().mockReturnValue(() => { }),
     } as unknown) as ReactSDKClient;
   });
 
-  it('throws an error when not rendered in the context of an OptimizelyProvider', () => {
+  it('does not throw an error when not rendered in the context of an OptimizelyProvider', () => {
     expect(() => {
       // @ts-ignore
       mount(<OptimizelyExperiment experiment="experiment1">{variation => variation}</OptimizelyExperiment>);
-    }).toThrow();
+    }).toBeDefined();
   });
 
   describe('when isServerSide prop is false', () => {

src/Feature.spec.tsx

@@ -46,10 +46,10 @@ describe('<OptimizelyFeature>', () => {
       onReady: jest.fn().mockImplementation(config => onReadyPromise),
       getFeatureVariables: jest.fn().mockImplementation(() => featureVariables),
       isFeatureEnabled: jest.fn().mockImplementation(() => isEnabledMock),
-      onUserUpdate: jest.fn().mockImplementation(handler => () => {}),
+      onUserUpdate: jest.fn().mockImplementation(handler => () => { }),
       notificationCenter: {
-        addNotificationListener: jest.fn().mockImplementation((type, handler) => {}),
-        removeNotificationListener: jest.fn().mockImplementation(id => {}),
+        addNotificationListener: jest.fn().mockImplementation((type, handler) => { }),
+        removeNotificationListener: jest.fn().mockImplementation(id => { }),
       },
       user: {
         id: 'testuser',
@@ -60,11 +60,12 @@ describe('<OptimizelyFeature>', () => {
       getIsUsingSdkKey: () => true,
     } as unknown) as ReactSDKClient;
   });
-  it('throws an error when not rendered in the context of an OptimizelyProvider', () => {
+
+  it('does not throw an error when not rendered in the context of an OptimizelyProvider', () => {
     expect(() => {
       // @ts-ignore
       mount(<OptimizelyFeature feature="feature1">{(isEnabled, variables) => isEnabled}</OptimizelyFeature>);
-    }).toThrow();
+    }).toBeDefined();
   });
 
   describe('when the isServerSide prop is false', () => {

src/Provider.tsx

@@ -1,5 +1,5 @@
 /**
- * Copyright 2018-2019, Optimizely
+ * Copyright 2022, Optimizely
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -66,7 +66,15 @@ export class OptimizelyProvider extends React.Component<OptimizelyProviderProps,
     }
 
     if (finalUser) {
-      optimizely.setUser(finalUser);
+      if (!optimizely) {
+        logger.error(`Unable to set user ${finalUser} because optimizely object does not exist.`)
+      } else {
+        try {
+          optimizely.setUser(finalUser);
+        } catch (err) {
+          logger.error(`Unable to set user ${finalUser} because passed in optimizely object does not contain the setUser function.`)
+        }
+      }
     }
   }
 

src/hooks.ts

@@ -1,5 +1,5 @@
 /**
- * Copyright 2022, Optimizely
+ * Copyright 2018-2019, 2022, Optimizely
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -67,7 +67,7 @@ interface UseExperiment {
   (experimentKey: string, options?: HookOptions, overrides?: HookOverrides): [
     ExperimentDecisionValues['variation'],
     ClientReady,
-    DidTimeout
+    DidTimeout,
   ];
 }
 
@@ -76,15 +76,15 @@ interface UseFeature {
     FeatureDecisionValues['isEnabled'],
     FeatureDecisionValues['variables'],
     ClientReady,
-    DidTimeout
+    DidTimeout,
   ];
 }
 
 interface UseDecision {
   (featureKey: string, options?: DecideHooksOptions, overrides?: HookOverrides): [
     OptimizelyDecision,
     ClientReady,
-    DidTimeout
+    DidTimeout,
   ];
 }
 
@@ -191,8 +191,10 @@ function useCompareAttrsMemoize(value: UserAttributes | undefined): UserAttribut
  */
 export const useExperiment: UseExperiment = (experimentKey, options = {}, overrides = {}) => {
   const { optimizely, isServerSide, timeout } = useContext(OptimizelyContext);
+
   if (!optimizely) {
-    throw new Error('optimizely prop must be supplied via a parent <OptimizelyProvider>');
+    hooksLogger.error(`Unable to use experiment ${experimentKey}. optimizely prop must be supplied via a parent <OptimizelyProvider>`);
+    return [null, false, false];
   }
 
   const overrideAttrs = useCompareAttrsMemoize(overrides.overrideAttributes);
@@ -212,6 +214,7 @@ export const useExperiment: UseExperiment = (experimentKey, options = {}, overri
       didTimeout: false,
     };
   });
+
   // Decision state is derived from entityKey and overrides arguments.
   // Track the previous value of those arguments, and update state when they change.
   // This is an instance of the derived state pattern recommended here:
@@ -283,8 +286,15 @@ export const useExperiment: UseExperiment = (experimentKey, options = {}, overri
  */
 export const useFeature: UseFeature = (featureKey, options = {}, overrides = {}) => {
   const { optimizely, isServerSide, timeout } = useContext(OptimizelyContext);
+
   if (!optimizely) {
-    throw new Error('optimizely prop must be supplied via a parent <OptimizelyProvider>');
+    hooksLogger.error(`Unable to properly use feature ${featureKey}. optimizely prop must be supplied via a parent <OptimizelyProvider>`);
+    return [
+      false,
+      {},
+      false,
+      false,
+    ];
   }
 
   const overrideAttrs = useCompareAttrsMemoize(overrides.overrideAttributes);
@@ -353,7 +363,12 @@ export const useFeature: UseFeature = (featureKey, options = {}, overrides = {})
     return (): void => { };
   }, [optimizely.getIsReadyPromiseFulfilled(), options.autoUpdate, optimizely, featureKey, getCurrentDecision]);
 
-  return [state.isEnabled, state.variables, state.clientReady, state.didTimeout];
+  return [
+    state.isEnabled,
+    state.variables,
+    state.clientReady,
+    state.didTimeout,
+  ];
 };
 
 /**
@@ -365,11 +380,21 @@ export const useFeature: UseFeature = (featureKey, options = {}, overrides = {})
  */
 export const useDecision: UseDecision = (flagKey, options = {}, overrides = {}) => {
   const { optimizely, isServerSide, timeout } = useContext(OptimizelyContext);
+
   if (!optimizely) {
-    throw new Error('optimizely prop must be supplied via a parent <OptimizelyProvider>');
+    hooksLogger.error(`Unable to use decision ${flagKey}. optimizely prop must be supplied via a parent <OptimizelyProvider>`);
+    return [
+      createFailedDecision(flagKey, 'Optimizely SDK not configured properly yet.', {
+        id: null,
+        attributes: {},
+      }),
+      false,
+      false,
+    ]
   }
 
   const overrideAttrs = useCompareAttrsMemoize(overrides.overrideAttributes);
+
   const getCurrentDecision: () => { decision: OptimizelyDecision } = () => ({
     decision: optimizely.decide(flagKey, options.decideOptions, overrides.overrideUserId, overrideAttrs),
   });
@@ -452,5 +477,9 @@ export const useDecision: UseDecision = (flagKey, options = {}, overrides = {})
     return (): void => { };
   }, [optimizely.getIsReadyPromiseFulfilled(), options.autoUpdate, optimizely, flagKey, getCurrentDecision]);
 
-  return [state.decision, state.clientReady, state.didTimeout];
+  return [
+    state.decision,
+    state.clientReady,
+    state.didTimeout,
+  ];
 };