createInstance can throw: how to graceful fallback

[fabb]

There are a few issues that make it impossible for the application to still work if optimizely fails to initialize:

1. optimizely.createInstance from optimizely-sdk can return null
   https://github.com/optimizely/javascript-sdk/blob/v4.7.0/packages/optimizely-sdk/lib/index.browser.ts#L136
2. createInstance in the react-sdk does not handle the null case and then just throws in the constructor when accessing this._client.onReady
   https://github.com/optimizely/react-sdk/blob/2.7.0/src/client.ts#L195-L201
3. if we catch this thrown error in our application code, we cannot render OptimizelyProvider because it needs an OptimizelyReactSDKClient instance
   https://github.com/optimizely/react-sdk/blob/2.7.0/src/Provider.tsx#L27
4. conditionally rendering the OptimizelyProvider is not an option because the optimizely hooks from react-sdk throw when there is no optimizely instance provided via the OptimizelyProvider
   https://github.com/optimizely/react-sdk/blob/2.7.0/src/hooks.ts#L332-L336
5. the rules of hooks disallow to render hooks conditionally, so we cannot catch this issue on the application side

We currently see in production ~70.000 failed optimizely initializations / hour. All happening in Firefox, and we need to investigate more, but the main issue is that failing optimizely makes the application unusable, which is not a graceful fallback.

I have different ideas to solve this:

1. react-sdk allows to render OptimizelyProvider with optimizely={null} and the optimizely hooks are modified to not throw anymore when no optimizely instance is available. Applications will need to catch the thrown error from the initial createInstance call and as a fallback use null. The hooks then fall back to the same values they would when optimizely is not yet ready.
2. react-sdk does not throw in createInstance when optimizely is null and rather wraps all accesses to this._client with a null check.
3. react-sdk provides a DummyReactClient that implements all the functions from ReactSDKClient with no functionality. Applications will need to catch the thrown error from the initial createInstance call and as a fallback use the DummyReactClient and provide it to OptimizelyProvider.

I would vote for option 1. because it still allows applications to detect when createInstance fails and allows for the maximum flexibility. Also it's backwards-compatible.

[Develliot]

+1 Your PR would save me a re-render I'm currently doing something like this:

export const OptimizelyWrapper: FC<{}> = ({ children }) => {

.....
  const client: ReactSDKClient = getOptimizelyInstance();
.....

  return (
    <>
      {client ? (
        <OptimizelyProvider
          optimizely={client}
          user={{
            id: userID,
            attributes: {
              user_id: userID,
              is_logged_in: !!fundraiser?.uniqueId,
            },
          }}
          isServerSide={!isBrowser}
        >
          {children}
        </OptimizelyProvider>
      ) : (
        <>{children}</>
      )}
    </>
  );
};

export default OptimizelyWrapper;

I shouldn't have to be doing this

[fabb]

@Develliot the problem is that with your code, the optimizely hooks throw when rendered without <OptimizelyProvider>: https://github.com/optimizely/react-sdk/pull/135/files#r793491878

[zashraf1985]

@fabb @Develliot We are picking this up and will try to fix it over the next few days. its a legit ask to add safeguard into React SDK to make sure optimizely-sdk returning a null client does not break the React App and we are picking it up for fixing. On the other hand, i would like to know more about the exact errors you see in the firefox consoles. I would like to investigate the reason why our javascirpt SDK is failing and returning a null client in the first place. Can you share some more information regarding that ?

[fabb]

Hi,
unfortunately I just see the errors in Sentry from real users, and could not reproduce the issue locally. I investigated quite a bit to find out what the underlying issue could be, and my strongest hypothesis is this: optimizely/javascript-sdk#720

[Develliot]

Why can't the provider just be handed the SDK key and deal with client instance all internally either SSR or async why is it a separate thing that we need to think about, it would be a much nice dev experience if we only need to think about the provider and the use decision hook.

[opti-jnguyen]

@fabb - hello! Thanks again for the suggestions, we've gone ahead and readied a PR to address this issue here: #149

This resolution will be part of the next minor version update of the SDK which should be coming soon. Once it's released we will update this issue and close it.

---

Also, wanted to repost this reply here in case anyone misses it on the PR:

@Develliot - We discussed this approach internally and due to the nature of the change, we will consider the change as a new feature request as it requires refactoring many components.

Our suggestion is to add it to our feature request board here - we will prioritize it based on the number of votes it gets!

Best,
John

cc: @zashraf1985

[opti-jnguyen]

@fabb - just released a new patch yesterday that should resolve this. Thanks again for reporting the issue!

• Release 2.8.1 · optimizely/react-sdk
• npm: @optimizely/react-sdk

cc: @zashraf1985

[fabb]

Nice, thanks!