LocalStorage bootsrap option takes over 100ms to initialize

[inssein]

Describe the bug Assumption is that if bootstrap: "localStorage" is passed in as options, then the variations will be available right away.

However, in timing this ourself, it is generally taking over 100ms for it to be available. This is an issue for us because on reload of our application, the user now has outdated options.

To reproduce Initialize LD with bootstrap option, and calculate time to ready event

Expected behavior Ready event should fire right away - we actually wish it could be synchronous, but the API might not allow for that.

SDK version 2.15.2

OS/platform Catalina, Chrome 78

[eli-darkly]

A couple of questions:

1. In the case where you're seeing a 100ms delay on startup, did the browser definitely already have flags in local storage for this user from a previous session? What were your exact test steps, starting with a clean browser state?

2. What do you mean by "on reload of our application, the user now has outdated options"? I mean, if you are using the local storage option then it is certainly possible to get outdated flags when you start up, because that is the whole point of local storage mode: it immediately gives you the last known values if there are any, so you don't have to wait for a request to LaunchDarkly. That is expected, and I'm not sure what the difference is between what you're seeing and how you want it to work, or what the 100ms delay has to do with the outdatedness of the flags. Again, could you be more specific about the scenario you have in mind, step by step?

Also:

we actually wish it could be synchronous, but the API might not allow for that

Unfortunately, it isn't possible for us to completely avoid asynchronous behavior in this case. There are just too many steps in initialization where we may need to do something async, so even if those async steps are not happening in your specific use case, there's no way to completely get rid of callback steps without completely duplicating all the rest of the initialization logic in a separate code path just for that use case.

Moreover, in local storage mode we don't know whether there really are any flags in local storage until you call the init method. If there aren't any, then it is going to have to be asynchronous anyway since it will have to get the flags from LD. So your application code cannot assume that the client will be initialized immediately.

[inssein]

1) It always takes over 100ms to initialize (seems like after a network call to LaunchDarkly) no matter if there is data in localStorage or not. Currently I can see that I have the flag state in my localStorage, but it doesn't seem to be used by the client library

2) I just meant that certain flags are used early on (right after initialization), and the localStorage aspect not working causes the wrong flag to be used until data from LaunchDarkly comes back.

[eli-darkly]

I'm having trouble seeing what the scenario is that is being described here. You still haven't provided any information about specifically how you are initializing the client (with what options), or exactly how you determined that there is flag state in local storage (note that the for the data is per-user, so just because you see something there doesn't mean it is the data the client would be using for this particular test run). I may need to refer you to the support team because honestly this sounds like more of a support request, where there is no clear evidence as to whether this is an SDK issue or an SDK usage issue.

From what I've seen so far, it sounds like the local storage mode is not actually in effect at all here. That is, you are saying the client always takes >100ms to initialize, and that it never finishes initializing until it has finished making a request to LaunchDarkly; that is exactly what its behavior would be by default, without the option.

But, again, in order to determine whether this is a case of the functionality just not working at all due to an SDK bug, or something being wrong in how the application is setting things up, I would need to know actual details of what you are doing. "To reproduce: Initialize LD with bootstrap option" is not really a description of steps to reproduce, and not surprisingly when I made up some test steps based on only that description, I was not able to reproduce the problem (that is, initialization takes less than 10ms for me after the first time). So if you want to pursue this here as a bug report, I'll need at minimum 1. the actual initialization options and user properties that you're using, 2. exactly what you saw that indicated the local storage data is there (and when you saw it, that is, if you start with a clean browser that does not have the data, does it show up immediately after you first your load your app), 3. what your code is doing after you call initialize (it sounds like you are waiting for the "ready" event... but since you said "certain flags are used early on (right after initialization) ... the wrong flag [is] used until data from LaunchDarkly comes back" it also sounds like you're not waiting for the event). You can also pursue this as a support request, but they would need the same information.

[eli-darkly]

It's been nearly a month since my last comment, so I will close this issue soon if there's no response.

[inssein]

@eli-darkly woah I think i missed that reply, my apologies.

I would love to try and create a codepen to explain the problem, is there a public codepen I could modify?

Before I go into some detail, lets use an example of a flag called NEW_MESSAGE_HEADER. In LaunhDarkly and in localstorage, its set to true, but in the application, it defaults false.

1. Initialization options

   
   import { initialize } from "launchdarkly-js-client-sdk";

const identity = { key: ${user.username}.${user.companyIdentifier}, name: fullName(user.firstName || "", user.lastName || ""), custom: { company: user.companyIdentifier, teamId: action.teamId, cluster: whatami.cluster } };

const client = initialize(launchDarklyClientSideId, identity, { bootstrap: "localStorage", fetchGoals: false });

2. When I clear all local storage, load the application, and then go to Chrome Inspector -> Application Tab -> Local Storage, I see the `ld:....:.....` local storage key with the relevant data that looks correct.

3. After I call initialize, I basically start rendering the application. 

**The Bug:** The application will initially render as if `NEW_MESSAGE_HEADER` is set to false, and once LaunchDarkly initializes, and the application re-draws (we use react), it renders with `NEW_MESSAGE_HEADER` set to true.

This causes a jarring experience for our users because the application re-renders with the new state every time, even when the updated state is in local storage.

I would expect that the client would initialize with the data from local storage from a previous load, but that doesn't seem to be happening.

[inssein]

Said a little differently, if I took the above code, and added console.time and console.timeEnd like so:

import { initialize } from "launchdarkly-js-client-sdk";

const identity = {
  key: `${user.username}.${user.companyIdentifier}`,
  name: fullName(user.firstName || "", user.lastName || ""),
  custom: {
    company: user.companyIdentifier,
    teamId: action.teamId,
    cluster: whatami.cluster
  }
};

const client = initialize(launchDarklyClientSideId, identity, {
  bootstrap: "localStorage",
  fetchGoals: false
});

// start measuring time
console.time("LD")

client.on("ready", () => {
  // record how long initialization took 
  console.timeEnd("LD");
});

My console consistently reports over 70ms.

[eli-darkly]

Well, that continues to support what I said in my last comment, which is that it looks like the client is just not seeing the local storage data at all when you start up. The elapsed time is a secondary side effect, since what you're measuring there is just the normal time it takes to contact LaunchDarkly if it does not have local storage data; the real way to confirm what's going on would be to modify the local storage data so it has a different flag value, then see whether it ever gets that value (which, based on your findings so far, I would presume it does not).

One other thing I'm unclear on, though:

After I call initialize, I basically start rendering the application.

The Bug: The application will initially render as if NEW_MESSAGE_HEADER is set to false, and once LaunchDarkly initializes, and the application re-draws (we use react), it renders with NEW_MESSAGE_HEADER set to true.

In your example, NEW_MESSAGE_HEADER is true in LaunchDarkly and true in the local storage data. If you're getting false, I think that means you are checking the flag value before the client is ready. I see that you're not waiting for the "ready" event or calling waitForInitialization() in your first code sample.

It's true that if it is able to use local storage data, then the client will become ready much faster. But it's not guaranteed to be instantaneous: there may still be asynchronous operations it needs to do internally, even if those don't involve any network requests, so you should not treat initialize as a synchronous operation that will return a fully usable client. The only case in which it would be safe to do that is if you were setting bootstrap to an object with a bunch of preset values.

Moreover, for a significant subset of users— the ones who haven't visited your site before— you will definitely not have local storage data at page load time, in which case what you're doing will definitely not work. I mean, you could check the contents of local storage programmatically and thereby know whether it's likely to be able to start up faster or not, but if there's nothing there, then you really have no option but to wait for the "ready" event... so why not just always wait for it?

That is a separate issue from the possible bug we're talking about, but I just wanted to be clear about the intended behavior.

[eli-darkly]

Anyway, I will continue trying to reproduce this. We don't have a codepen but if there's any other sample code you want to provide, put it wherever you want. If there's anything you don't want to post publicly, either email me or contact support.launchdarkly.com.

[inssein]

In my example, NEW_MESSAGE_HEADER is true in LaunchDarkly and true in local storage, but because LaunchDarkly doesn't seem to utilize the local storage data at all, it only becomes true after contacting LaunchDarkly (code default is false).

I don't really know what else to do beside read the local storage value launch-darkly already has, and then pass it in as the bootstrap option. It seems like the key is ld:${client_id}:${base64(identity)}.

How else can I debug why the local storage options aren't being picked up?

As for the last issue, we are aware and still not perfect, but at least it doesn't happen on every single load.

[inssein]

I have spent a little time on my side debugging this, stepping through minified code haha, but it does seem like LaunchDarkly is working the way it is intended to. I think there is just so much going on at the beginning of the application (so many things initializing) that it just ends up taking LD that long to respond with "ready".

Edit: Not important right now, but I wonder if it's worth making this process synchronous for those who need it. Right now it seems like the process is unnecessarily async (setTimeout(fn, 0)).

[eli-darkly]

@inssein As I said, I am still trying to reproduce this. I don't know how to answer your question "How else can I debug why the local storage options aren't being picked up?"; if I had an idea yet of what is going on, I would have shared it. However, when you say—

it does seem like LaunchDarkly is working the way it is intended to. I think there is just so much going on at the beginning of the application (so many things initializing) that it just ends up taking LD that long to respond with "ready"

—that seems very implausible to me. You said that you're seeing a network request, and that the client does not finish initializing until after the network request. I don't know what you mean by "so many things initializing" (if you've been debugging through the code in detail, it'd be helpful if you described what you saw more specifically) but there just isn't anything it is doing that could take 100ms other than network I/O, and the whole point of local storage mode is that if the client did have local storage data, then it is not supposed to wait for the network request; otherwise there would be no point to this feature at all. So I'm completely convinced that local storage mode is not working correctly in this case, for some reason, and that is the entire explanation for the delay you're seeing.

Also, I'd like to re-emphasize that even if everything was working 100% correctly it would not be correct to try evaluating flags before waiting for the "ready" event (or awaiting the waitForInitialization() promise). When you say the flag "only becomes true after contacting LaunchDarkly (code default is false)", that is a condition your application must be able to tolerate, because many users will not have any local storage data yet. Even though the delay of connecting to LD may be annoying, it will definitely be necessary some of the time, and it will always be the case that the LD client will return a default value (false) if it has not been given a chance to initialize yet.

[eli-darkly]

Also, about this point:

I wonder if it's worth making this process synchronous for those who need it. Right now it seems like the process is unnecessarily async

Unfortunately, it can't be any more synchronous than it currently is. There are two reasons:

1. As I've said before, even if you are using local storage mode, you cannot assume that the user already has local storage data. So the client has to have the option of connecting to LD, which can't possibly work unless we are using an async/callback mechanism. Therefore, if there is any chance that the client will need to connect to LD— which it definitely will need to at some point at least once for any new user, since it has not yet had a chance to put any data into local storage— the application must use async semantics to initialize the client.

The only case where it's safe for the caller to assume that the client will initialize immediately is if the caller passes in all of the flag values in a bootstrap object. We know in that case, just by looking at the input parameters, that we will not have to connect to LD.

2. When you see the pattern of setTimeout(fn, 0), that is not for no reason. It's a JavaScript best practice that any function that uses callback semantics, if it may need to defer the callback via the event loop (that is, if there is any chance that it will need to actually do anything asynchronous— and if there wasn't, then there would be no reason to use a callback in the first place), should guarantee that it always does so. This is because event-driven callbacks are such a common pattern in JS that developers often assume (reasonably) that if they write something like this—

    doSomethingAsynchronous(someParameter, function() {
        theOperationHasFinished();
    });
    theOperationHasStarted();

—then theOperationHasFinished() will definitely not be called prior to theOperationHasStarted(). So, if doSomethingAsynchronous sometimes needs to really do something asynchronous and sometimes doesn't, then the correct implementation is this—

    function doSomethingAsynchronous(someParameter, callback) {
        if (weReallyNeedToDoIOForThis(someParameter)) {
            doTheIO(someParameter, function(result) {
                callback();
            });
        } else {
            setTimeout(callback, 0);
        }
    }

—and not this:

    function doSomethingAsynchronous(someParameter, callback) {
        if (weReallyNeedToDoIOForThis(someParameter)) {
            doTheIO(someParameter, function(result) {
                callback();
            });
        } else {
            callback();
        }
    }

Now, there are some internal use cases in the SDK where those concerns don't apply— that is, if we know that this code is only called from places in our own code where we're definitely not relying on the execution order, then it might be OK to call the callback immediately. However, that makes no difference from the point of view of the application code, since they will still need to provide a callback to whatever higher-level SDK function they're calling, so we will still need to defer that callback.

In the specific case of the local storage logic, you may wonder why the internal API for this uses callbacks at all, since local storage operations in the browser are synchronous. The answer is that the same JavaScript client code is also used in non-browser environments where there are only async operations for storage (for instance, in LaunchDarkly's client-side Node SDK and Electron SDK), so we have to use an abstraction that allows for the option of async behavior. But, again, getting rid of that abstraction would not mean that you (the app developer) could call the client API without a callback, because as long as there may be async things we need to do, we must use callback semantics.

[eli-darkly]

Unfortunately, since we haven't been able to reproduce this with the conditions that you provided— that is, for us, a simple application that just initializes the client with "bootstrap":"localStorage" works as expected when running in Chrome with JS SDK 2.15.2; our unit tests for this feature are all working as well, and no other customers have reported a problem like this— I have to assume that either there's something special in your code that we're missing, or else there's something unusual in your environment. I don't know what that would be. If browser local storage operations were not working at all (e.g. if cookies are disabled— the browser treats local storage permissions the same as cookie permissions) then it would not have been able to store data there, and you said that you did see some stored data. So assuming you didn't disable cookies in between the original run and your later test run, I don't know how to explain that.

[eli-darkly]

Would you mind trying to reproduce this with the smallest possible test code: the hello-js app, minimally modified to add your environment ID and flag key and the "bootstrap":"localstorage" option? And if you see the same behavior there, please email me the exact code that you ran.

[inssein]

@eli-darkly I think I have confirmed that this is on our side, but I can try the hello-js app when I get some cycles.

Proof:

const client = initialize(launchDarklyClientSideId, identity, {
  bootstrap: "localStorage",
  fetchGoals: false
});

console.time("HELLO");
setTimeout(() => console.timeEnd("HELLO"), 0);

client.on("ready", () => {
  // ready
});

Currently, my console is returning a time of > 200ms, just to get to the next event loop (HELLO: 232.19189453125ms).

We run a pretty large single page application, and as I mentioned, there is a lot going on at initialization (lots of remote calls, lots of javascript being loaded and parsed, etc).

[eli-darkly]

To be clear, are you saying that your comment about "so much going on at the beginning" was referring to things your application code is doing, not initialization work within the SDK? If so... then I'm still a bit confused, because if it takes that long just to set up the app's own environment, I'm unclear on how you would expect to be able to start rendering pages immediately if the LD client had been available immediately.

[inssein]

Yes, so many things going on with the application.

We have a large react-based single page application. Since the LD client is asynchronous, most of the time after the LD.initialize() call seems to be made to render the application, and when the event loop fires again (and "ready" fires), there has already been a first paint of the application.

[drslump]

we are also hitting this issue with a Nuxt app, it initializes with a very hot loop to hydrate the page DOM, I would assume that this is true for most SSR schemes, so the problem manifest by how the framework runs its startup process and how launchdarkly reads localStorage from a timeout:

• Nuxt runs its plugins, which is where people would usually place the launchdarkly setup
• Nuxt gets busy, without yielding the execution frame, rendering the app
• Most of the app logic has executed at this point with the launchdarkly flags empty :(
• Nuxt finishes its startup and then timers can finally start executing
• Launchdarkly's timeout fires and it loads from localStorage, triggering the ready event
• The app was already rendered without flags at this point

All this would be solved if launchdarkly loaded localStorage in the same execution frame that its setup (call to initialize). That way the whole app startup would already see those cached flags. I find the current behaviour surprising to be honest.

In pseudo code, this is what I would expect to work:

const ld = initialize(..., { bootstrap: localStorage })
ld.allFlags() // obtains the cached flags
ld.on('ready', () => {}) // fires on next microtask

The workaround is easy enough, we'll just handle the cache ourselves.

[eli-darkly]

I'm closing this because I don't think there is any clear action for us to take at this point. The last comment from @drslump reiterates that people would prefer for initialize to be synchronous if you're using local storage caching, but I already did my best to explain at some length why we can't do that (short version: there are several reasons, but it's primarily because you have to assume that there may not already be cached data present when you call initialize, in which case it'll have to make an HTTP request, which can't possibly be synchronous).