Method `waitForInitialization()` will never resolve when LaunchDarkly APIs fail due to 5XX

[jpinho]

trafficstars

Describe the bug Method waitForInitialization() will never resolve when LaunchDarkly APIs fail due to 5XX

To reproduce Mock one of your APIs into 5XX, sample with msw below

    server.use(
      http.get(
        'https://stream.launchdarkly.com/all',
        () => {
          return new HttpResponse('bad response', {
            status: 500,
            headers: {
              'Content-Type': 'text/plain',
            },
          });
        },
        { once: true },
      ),
    );

Expected behavior LD clients don't want to wait forever for a FF service. Therefore a timeout must exist to let applications bail out of the LD waitForInitialization method.

I can obviously do a Promise.race on my side as below, but such logic should definitely be part of the SDK:

export const initDarklyClient = async () => {
  const darklyClient = getDarklyClient();

  if (darklyClient) {
    return await Promise.race([
      new Promise((resolve, reject) =>
        setTimeout(() => {
          if (darklyClient.initialized() && !darklyClient.isOffline()) {
            resolve('Darkly client initialized');
          } else {
            reject(new Error('Darkly client initialization timeout'));
          }
        }, LAUNCH_DARKLY_INIT_TIMEOUT),
      ),
      darklyClient.waitForInitialization(),
    ])
      .then(() => {
        Log.info('process-docx-handler:worker:darklyClient ready');
      })
      .catch((err) => {
        Log.warn('process-docx-handler:worker:darklyClient failed to initialize');
      });
  }

  return Promise.reject('Darkly client failed to initialize');
};

I can provide a contribution, if we have an agreement here.

SDK version

"launchdarkly-node-server-sdk": "^7.0.3",

Language version, developer tools

• node 18

OS/platform

• macOS

[kinyoklion]

Hello @jpinho,

Thank you for the issue.

We do recommend using promise.race currently to control the user specified timeout.

Something more like:

  await Promise.race([
      client.waitForInitialization(),
      new Promise((resolve) => setTimeout(resolve, 3000)),
    ]);

If waitForInitialization isn't returning immediately for an offline client, then that is problematic. Multiple calls to client.waitForInitialization should also be fine because it caches a promise that remains in the resolved state.

The SDK only fails to initialize under certain situations, a 500 is not such a condition and it will continue to retry using an exponential backoff with jitter. Something like a 404 would be a terminal condition.

I've entered the issue as well, as some people would prefer a built-in timeout.

As a reminder this repository is not the home of development for the launchdarly node SDK at this point in time.

The current node SDK implementation is here: https://github.com/launchdarkly/js-core/tree/main/packages/sdk/server-node

With the latest version being the 9.x release.

Thanks, Ryan

Filed internally as 231798

[jpinho]

Thanks @kinyoklion for your answer! I made some points here as well https://github.com/launchdarkly/node-server-sdk/issues/288#issuecomment-1943667112

Touching the same point again, LD resiliency should definitely be improved:

1. A FF client should never throw runtime exceptions ever, for any reason; instead Log.error should be preferred, and if failing to evaluate a flag, then by default false should be returned. Up to the applicational code to deal with that then.

2. An initialisation attempt on a given timeout, should be provided by the client itself 💯

Not complying to these two points, means customers such as myself at epilot, have to come up with their own Shared Library of epilot-launch-darkly-sdk (a wrapper) that adds resiliency and helps us avoid having to repeat that resiliency code across our over 10+ micro services.

Would love to contribute to the codebase if it's open to outside contributors. Let me know how.

[kinyoklion]

@jpinho This, and the new repository, are open source and have an included CONTRIBUTORS guide you can reference.

Adding a timeout to initialization I would be open to, and we do support in some SDKs.

[kinyoklion]

Please note this repository itself isn't where you would make the contribution. It would be here: https://github.com/launchdarkly/js-core/tree/main/packages/sdk/server-node

[kinyoklion]

Version 9.4.0 and newer of @launchdarkly/node-server-sdk now supports an optional timeout in waitForInitialization. Additionally in a future (major) version the timeout will start to have a default or will not be optional.