react: possible race condition between init and identify
When debugging a hard to reproduce issue where flagsmith.flags was randomly {}, causing the wrong states to be rendered in our react app, it turned out to be related to a race between flagsmith.init (called implicitly by the FlagsmithProvider) and flagsmith.identify (called explicitly if the user is logged in). The bug would occur sometimes when init didn't finish when calling identify, so our fix looks something like this:
// module level
const initPromise = flagsmith.init({ environmentID, defaultFlags });
// when user is identified (e.g. inside some component hook)
useEffect(() => {
if (user) {
const identify = async () => {
await initPromise;
await flagsmith.identify(user.id, user.traits);
};
identify();
}
}, [flagsmith, user]);
// render FlagsmithProvider without options!
const App = () => (
<FlagsmithProvider flagsmith={flagsmith}>
IMO the library should ensure these races don't happen in the first place, possibly by storing an init Promise internally and awaiting it whenever identify is called (or other methods requiring initialization to be finished).
Thanks for raising this @cbix. We will look into this and provide updates here.
Hi @cbix, we are working on a solution for this, in the meantime, you could also send an identity with the traits when initializing Flagsmith like this: https://docs.flagsmith.com/clients/javascript#example-initialising-the-sdk-with-a-user
I've also been having trouble with this race condition. @matthewelwell 's fix worked for me:
- Remove the options from the provider
- Manually initialize at the top of the top of the file, storing the returned promise.
- Await that promise before calling identify.
Passing the identity with initializing Flagsmith is not an option because identifying the currently logged in user is an asynchronous process and we don't want to wait for that to finish before rendering the app (including initializing Flagsmith).
Just to summarise the issue so I can action it today, are we essentially saying that the identity flags are being overwritten by a late returning init call?
I think if that's the case perhaps the optimal solution is for the SDK to just ignore the response rather than await init since we're going to be ignoring init anyway.
@kyle-ssg I can't tell for sure what's causing the flags to be set to {} but the race works like this
init()is called async, performs some long-running tasks like fetchidentify()is called and finishes- The
init()call from (1) is still running and now performs some tasks that cause flags to be{}
The workaround basically synchronizes init and identify, so (3) can never happen after (2), i..e. identify only runs after init finished.
Thanks for the quick response. Ok I'll replicate this in a test and message back here when I've got to the bottom of it.
Thanks, good luck with the replication, race conditions are by nature hard to reproduce 💁
Ok! So I've replicated that exact scenario via a test and fixed here.
I've published this under [email protected], if you are able to replicate this consistently enough it would be amazing if you could confirm this version solves your issue.
cc @cbix @KaidenR
@kyle-ssg I won't be able to test this until Sep 24 but if for a sufficient number of test runs it sometimes failed before and passed after the fix, it should be good. The patch just doesn't look like it's properly synchronizing the code, but if it's handling everything that could go wrong in case of a race, there's nothing wrong with that :)
The patch just doesn't look like it's properly synchronizing the code, but if it's handling everything that could go wrong in case of a race, there's nothing wrong with that :)
By this do you mean identify doesn't await init? If so that was by design, would rather it resolve quicker and ignore the api response of init.
This is building under flagsmith@ 4.1.2, please reopen this issue if any related problem persists. Thanks all for being active in replying!
It looks like the fix works for me:
- I reverted my own fix (awaiting the init promise before calling identify)
- I reproduced the error (all flags missing)
- I upgraded to
[email protected]and restarted my Vite server - Flags are loading correctly now.
Thanks for the crazy-fast turnaround on this!