launchdarkly / js-core

LaunchDarkly monorepo for JavaScript SDKs
Other
15 stars 19 forks source link

Issue running with the example code from the node redis integration. #568

Closed kikar closed 2 months ago

kikar commented 2 months ago

Describe the bug The example provided in the README.md is not working

To reproduce

import { init } from '@launchdarkly/node-server-sdk';
import { RedisFeatureStore } from '@launchdarkly/node-server-sdk-redis';

(async function main() {
  const client = init(process.env.LD_KEY!, {
    featureStore: RedisFeatureStore(),
  });
  await client.waitForInitialization({ timeout: 5 });
})();

Expected behavior Not to throw errors

Logs

~/Developer/test/node_modules/@launchdarkly/js-server-sdk-common/dist/store/serialization.js:42
    if (value.generated_includedSet) {
              ^

TypeError: Cannot read properties of undefined (reading 'generated_includedSet')
    at Array.replacer (~/Developer/test/node_modules/@launchdarkly/js-server-sdk-common/dist/store/serialization.js:42:15)
    at JSON.stringify (<anonymous>)
    at serializeFlag (~/Developer/test/node_modules/@launchdarkly/js-server-sdk-common/dist/store/serialization.js:248:17)
    at Object.serialize (~/Developer/test/node_modules/@launchdarkly/js-server-sdk-common/dist/store/persistentStoreKinds.js:45:70)
    at visit (~/Developer/test/node_modules/@launchdarkly/js-server-sdk-common/dist/store/sortDataSet.js:34:24)
    at topologicalSort (~/Developer/test/node_modules/@launchdarkly/js-server-sdk-common/dist/store/sortDataSet.js:40:9)
    at ~/Developer/test/node_modules/@launchdarkly/js-server-sdk-common/dist/store/sortDataSet.js:59:40
    at Array.forEach (<anonymous>)
    at sortDataSet (~/Developer/test/node_modules/@launchdarkly/js-server-sdk-common/dist/store/sortDataSet.js:57:26)
    at ~/Developer/test/node_modules/@launchdarkly/js-server-sdk-common/dist/store/PersistentDataStoreWrapper.js:107:54

SDK version

Language version, developer tools Node: 20.17.0

OS/platform Mac OS Sonoma 14.6.1 (23G93)

kinyoklion commented 2 months ago

Hello @kikar,

Thank you for the report. I am able to run the example code without issue.

Is this an existing project that is being updated? A new project with just that confiuration?

Are you using a redis instance that is already initialized with some data from a previous version?

What package manager are you using, yarn, npm, etc?

Does the SDK initialize correctly when you do not use the redis intregration?

My initial suspicion would be to ensure that the dependencies for the common libraries are also matching the expected versions.

I've attached the lockfile from running the example. package-lock.json

Thank you, Ryan

kikar commented 2 months ago

Hi @kinyoklion , thank you for the reply. I've added a small example repo, including a compose file for redis: https://github.com/kikar/ld-redis-test

kinyoklion commented 2 months ago

@kikar

I tried your example in a Github codespace and it worked fine.

It had:

@kinyoklion ➜ /workspaces/ld-redis-test (master) $ node --version
v20.16.0
@kinyoklion ➜ /workspaces/ld-redis-test (master) $ npm --version
10.8.1

I do see you are using macOS, but that shouldn't be causing a problem like this.

Can you let me know if it works in a code space? If it does, then this would help to eliminate your specific flag configuration from being the source of the problem, and instead indicate a problem with your development environment.

Thank you, Ryan

kikar commented 2 months ago

@kinyoklion I added docker compose full reproduction steps, and still doesn't work. That should take out of the problem my machine (unless related to arm architecture). Check the README

Does it reproduce for you with the docker compose up command?

I will try to use the node/npm versions specified by your test

kinyoklion commented 2 months ago

I used docker compose up in the codespace.

What is in the LD environment you are using for testing? Is this an existing environment populated with flags, empty, etc?

kikar commented 2 months ago

Is our company testing env, already populated with flags

kinyoklion commented 2 months ago

@kikar

The only difference would be the project/environment. image

Would it be possible for you to try a different project? If a different project works, and your companies test environment does not, then you will need to raise a support ticket. We would need to see what data is causing the misbehavior.

Thank you, Ryan

kinyoklion commented 2 months ago

In the interim I can investigate and see if some additional safety checks are warranted.

kikar commented 2 months ago

@kinyoklion I created a new project, and now the SDK key works!

image

kinyoklion commented 2 months ago

@kikar

Thank you!

I figured out what the likely problem is and will make a patch.

The issue is encountered with JSON flags which are arrays and which contain null/undefined values.

[
  1,
  null,
  "potato"
]

It doesn't happen in most environments because this is not very typical. It also only happens with a persistent store integration because the SDK is re-serializing values to store in persistence.

Thank you for your report and help.

Ryan

kinyoklion commented 2 months ago

Filed internally as 255288

kikar commented 2 months ago

Amazing, thanks Ryan! That array with null was probably created by me since null is not accepted as valid JSON. Can you tackle this problem as well?

image

Will you update this ticket once the fix will be released?

kinyoklion commented 2 months ago

@kikar

The ticket will update when the fix PR is merged, and then I will release shortly after that. I can make a comment on the ticket at that time as well.

I think it would be best to start with support for your account representative for the null variation setting. Currently the JSON variations only support arrays or objects. If you have problems with that route, then let me know and I will see what I can do.

Thank you, Ryan

kinyoklion commented 2 months ago

@kikar I have released @launchdarkly/node-server-sdk version 9.5.4 which includes the fix for this issue.

Thank you, Ryan