statsig-io / js-client-monorepo

ISC License
4 stars 2 forks source link

updateUserAsync is not updating client's internal user value (async initialization - not awaited) #13

Closed kprice-ae closed 1 month ago

kprice-ae commented 1 month ago

Summary

We are using the Asynchronous Initialization (Not Awaited) strategy. Calling updateUserAsync does not update the Statsig Client's internal user value. The same goes for calling updateUserSync.

To verify this, I set up our client to log on any values_updated events, logged any calls to updateUserAsync, and wrapped this updateUserAsync call in a try / catch to log any errors. With a cleared cache,

Client setup

// initialize client with "Asynchronous Initialization (Not Awaited)" strategy
// see https://docs.statsig.com/client/javascript-sdk/init-strategies#3-asynchronous-initialization---not-awaited
const MyStatsigClient = new StatsigClient(
  STATSIG_SDK_KEY,
  // this is undefined on initial load after clearing cache
  { userId: USER_ID_FROM_LOCAL_STORAGE || undefined },
  {
    environment: {
      tier: ENV,
    },
    overrideAdapter: new StatsigOverrideAdaptor(),
  },
);

MyStatsigClient.initializeAsync().catch((error) => {
  console.error('Error initializing Statsig Client:', error);
});

// log any `values_updated` events
// see events at https://github.com/statsig-io/js-client-monorepo/blob/main/packages/client-core/src/StatsigClientEventEmitter.ts
const onValuesUpdated = (event: StatsigClientEvent<'values_updated'>) => {
  console.log('VALUES UPDATED CLIENT EVENT\n', event);
};
MyStatsigClient.on('values_updated', onValuesUpdated);

Calling updateUserAsync

export const useResolveStatsigUser = (): { statsigUser: StatsigUser; isLoading: boolean } => {
  // all three of the following methods of updating user exhibit the same behavior
  // const { updateUserSync } = useStatsigUser();
  // const { updateUserAsync } = useStatsigUser();
  const { client } = useStatsigClient();
  const { user, isLoading: isAppUserLoading } = useAppUser();

  const statsigUser = useMemo(() => {
    return {
      userID: user?.id,
      // ...custom fields excluded for simplicity
    };
  }, [user]);

  useEffect(() => {
    async function updateUser() {
      try {
        console.log('UPDATING STATSIG USER\n', statsigUser);
        // both methods of updateUserAsync exhibit the same behavior
        // await updateUserAsync(statsigUser);
        await client.updateUserAsync(statsigUser);
      } catch (error) {
        // no errors are logged
        console.error('Error updating statsig user', error);
      }
    }

    // eslint-disable-next-line @typescript-eslint/no-floating-promises
    updateUser();
  }, [user, statsigUser]);

  return {
    statsigUser,
    isLoading: isAppUserLoading,
  };
};
tore-statsig commented 1 month ago

Its still a bit unclear to me what you are saying is the bug vs what is expected. The gate/experiment values for the updated user are not correct, or the user itself is not set?

This is also quite a strange setup - not awaiting initialize but awaiting the user update, which is going to trigger immediately in a use effect, and returning a loading state that doesnt actually capture that sync action. I think you should just be awaiting the first initialization and not triggering updateUser right away

kprice-ae commented 1 month ago

Its still a bit unclear to me what you are saying is the bug vs what is expected. The gate/experiment values for the updated user are not correct, or the user itself is not set?

The initialization correctly sets a user if we pass in a StatsigUser from local storage on initialization. But user itself is not updated when calling updateUserAsync or updateUserSync. I expect the StatsigClient user to be updated when calling either of these.

This is also quite a strange setup - not awaiting initialize but awaiting the user update, which is going to trigger immediately in a use effect, and returning a loading state that doesnt actually capture that sync action.

The original implementation actually calls updateUserSync so that we don't await the user update. However, this still exhibits the same behavior (I attempted awaiting updateUserAsync to see if this would resolve the issue, but it did not).

The hook in my provided example is triggered on page load (if the user or any other custom properties have changed). There are other custom StatsigUser properties that we update in this hook, but I have excluded those in the provided example for simplicity-sake.

I think you should just be awaiting the first initialization and not triggering updateUser right away

We decided to go with this initialization strategy so that we don't have to await the promise from initialization (as described here: Asynchronous Initialization (Not Awaited) strategy

daniel-statsig commented 1 month ago

Hmmm, I'm not able to reproduce.

The first thing updateUserSync and updateUserAsync do is call _resetForUser which changes the clients internal user (https://github.com/statsig-io/js-client-monorepo/blob/main/packages/js-client/src/StatsigClient.ts#L415).

Can you share how you are checking the StatsigClient's user? Is it part of the useAppUser hook?

I ran the following but am seeing the user get changed:

const user = { userID: 'initial-user' };
const client = new StatsigClient(STATSIG_CLIENT_KEY, user);
client.on('values_updated', (e) => {
  console.log('Event: ', client.getContext().user, e);
});
client.initializeAsync().catch(console.error);

function Content() {
  const { user } = useStatsigUser();
  const [tick, setTick] = useState(0);

  useEffect(() => {
    setTimeout(() => {
      client
        .updateUserAsync({ userID: `updated-user-${tick}` })
        .catch(console.error);

      setTick(tick + 1);
    }, 1000);
  }, [tick, user.userID]);

  return <div>{user.userID}</div>;
}

export default function App(): ReactNode {
  return (
    <StatsigProvider client={client}>
      <Content />
    </StatsigProvider>
  );
}

This is resulting in: Screenshot 2024-08-08 at 10 35 33 AM

This utilizes the two way you can get the internal user, useStatsigUser or client.getContext()

kprice-ae commented 1 month ago

I seemed to have figured out the issue. Basically, we were updating the user (in useResolveStatsigUser) on initial render outside of the StatsigProvider. Moving this inside the StatsigProvider fixed this issue. I believe this is because updating the user outside of the provider will not cause the client in StatsigProvider to rerender.

We used to update the user before the StatsigProvider because the provider used to take the user as input before the latest SDK update. So, we would update the user as part of our useResolveStatsigUser hook and pass that user into the provider.

Before fix:

// initialize client with "Asynchronous Initialization (Not Awaited)" strategy
// see https://docs.statsig.com/client/javascript-sdk/init-strategies#3-asynchronous-initialization---not-awaited
const MyStatsigClient = new StatsigClient(
  STATSIG_SDK_KEY,
  { userId: USER_ID_FROM_LOCAL_STORAGE || undefined },
  {
    environment: {
      tier: ENV,
    },
  },
);

MyStatsigClient.initializeAsync().catch((error) => {
  console.error('Error initializing Statsig Client:', error);
});

function SetupStatsig() {
  const { user } = useResolveStatsigUser(); // this calls updateUser

  return <StatsigProvider client={MyStatsigClient}>{user.userID}</StatsigProvider>;
}

export default function App(): ReactNode {
  return (
    <SetupStatsig>
      <Content />
    </SetupStatsig>
  );
}

After fix:

// initialize client with "Asynchronous Initialization (Not Awaited)" strategy
// see https://docs.statsig.com/client/javascript-sdk/init-strategies#3-asynchronous-initialization---not-awaited
const MyStatsigClient = new StatsigClient(
  STATSIG_SDK_KEY,
  { userId: USER_ID_FROM_LOCAL_STORAGE || undefined },
  {
    environment: {
      tier: ENV,
    },
  },
);

MyStatsigClient.initializeAsync().catch((error) => {
  console.error('Error initializing Statsig Client:', error);
});

function SetupStatsig() {
  const { user } = useResolveStatsigUser(); // this calls updateUser
  return <>{children}</>;
}

export default function App(): ReactNode {
  return (
    <StatsigProvider client={MyStatsigClient}>
      <SetupStatsig>
        <Content />
      <SetupStatsig>
    </StatsigProvider>
  );
}