launchdarkly / node-server-sdk

LaunchDarkly Server-side SDK for Node
Other
79 stars 65 forks source link

getFlagReason always undefined with allFlagsState response #235

Closed ClementLevesque closed 2 years ago

ClementLevesque commented 2 years ago

Is this a support request? No.

Describe the bug

The method allFlagsState returns a Promise resolved by an object on which getFlagReason(key) returns always undefined :

const flagsState = await featureFlagClientInt.allFlagsState({ key });

// flagsState.getFlagReason(key) === undefined
// flagsState.allValues() & flagsState.getFlagValue() are ok 

Whereas featureDetail which returns the proper object :

const { reason, value } = await featureFlagClientInt.featureDetail(featureKey, { key }, false);
// reason.kind === 'OFF' || 'RULE_MATCH' etc.

To reproduce

Expected behavior getFlagReason should return a LDEvaluationReason object type.

Logs See above.

SDK version Current sdk version: launchdarkly-node-server-sdk: "5.13.4"

Language version, developer tools NodeJS 16.1.0

OS/platform On local env, Mac book pro M1.

Additional context Add any other context about the problem here.

kinyoklion commented 2 years ago

Hello @ClementLevesque,

By default allFlagsState does not include reasons for all flags. You can change the behavior by using LDFlagsStateOptions to indicate you want reasons.

await ldClient.allFlagsState({key}, {withReasons: true});

Thank you, Ryan

eli-darkly commented 2 years ago

Yes, that's what the API docs for getFlagReason mean when they say "it will be null if reasons were not recorded". Reasons are only recorded if you use the option Ryan mentioned.

As background, here's why the default is not to capture reasons. The most common use case for allFlagsState is to produce bootstrap data for initializing the JS SDK. That data will have to be passed in some way to the front end, such as by embedding it in a hidden HTML component, so it's desirable to not make it any bigger than necessary. And most front-end code that uses the JS SDK does not use evaluation reasons— getting them is an opt-in feature for client-side SDKs, even if you are connecting directly to LD and not using bootstrap data. So the reasons are omitted from that data by default.

ClementLevesque commented 2 years ago

It works ! Thanks a lot for your help and precious and detailed answer. I have to admit that the API docs could be more explicit about this option 😉