Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fallback Support for JavaScript #8009

Open
exhuma opened this issue Aug 28, 2024 · 4 comments
Open

Fallback Support for JavaScript #8009

exhuma opened this issue Aug 28, 2024 · 4 comments
Assignees

Comments

@exhuma
Copy link

exhuma commented Aug 28, 2024

Describe the feature request

Add support for "fallback" values to the JS API

Background

The documentation of the Python SDK mentions fallback values. This support is not present in the current JS SDK

Currently the isEnabled method cannot return a stable default value as recommended in your best practices. Some features may make sense to be "enabled by default" while others may need to be "disabled by default".

If the state cannot be retrieved for any reason, the current JS-SDK only ever returns false.

In a worst-case scenario this would leave application devoid of much functionality whenever unleash is unreachable or when features are not found for whatever reason.

Solution suggestions

Provide the same solution as provided in the Python SDK.

For example:

const client = new UnleashClient(...);
client.isEnabled('my-feature', () => true);

Alternatively, a static value would work too but this would break consistency with the Python SDK:

const client = new UnleashClient(...);
client.isEnabled('my-feature', true);
@ivarconr ivarconr self-assigned this Aug 29, 2024
@ivarconr
Copy link
Member

Hi @exhuma,

thanks for raising this and proposing a solution.

The Frontend SDKs works slightly different in Unleash, as they by default only knows about enabled feature flags for the provided context.

There are two main reasons for this:

  1. Do not expose flags not relevant for frontend.
  2. reduce payload as much as possible.

Because of this the frontend SDK will not be able to distinguish between "flag disabled" and "flag missing", which is the purpose of the fallback-value supported by server-side SDKs.

What we have done for the worst-case scenario you describe is the bootstrap support. This way to can bootstrap the SDK that will take precedence in the case the SDK is not able to connect back to the Unleash API:
https://github.com/Unleash/unleash-proxy-client-js?tab=readme-ov-file#bootstrap

Does the fallback mechanism solve your needs?

@exhuma
Copy link
Author

exhuma commented Aug 29, 2024

I think this solves it partially. I tested two situations:

  1. Behaviour when unleash is unreachable: Here, using "bootstrap" works as intended. It uses the bootstrapped value as "default".
  2. Behaviour when unleash is reachable but the feature flag is not (yet) known by unleash: Here, it does not work as intended. I see in the logs that initially the value is seen as true (as specified in the bootstrap) but then immediately reverts to false when using isEnabled.

It would be nice to have an "unknown" state to let the application decide what to do in that case.

I understand the reasoning why the front-end SDK only sees a "true" and "false". Especially for applications with a large number of feature flags. One way (but inefficient) to support the "unknown" state would be to sync all feature-flags on application startup (f.ex. when UnleashClient is initialised).

I've been thinking about a more efficient solution leveraging the polling process but this only sees information whenever a flag changes. For an established/live application there will be no changes and the polling always returns a 304 Not Modified response. So the local cache can never be properly "initialised".

I don't see a better solution than the one you currently implemented. The bootstrap is good for an unreachable unleash instance but it's not usable for feature-flags that are not yet known .

I understand that, because of these constraints having an "unknown" state is currently not possible. If you like you can close this issue.

@ivarconr
Copy link
Member

ivarconr commented Aug 29, 2024

thanks for the update. I will leave it open for a bit. This is an interesting use-case, but it does entail that we re-architect a bit on what flags are shared with frontend SDKs.

One high level thought would be to mark frontend-flags when you configure them (or possibly at the project level). However, this would require deep analysis to not break existing usage of Unleash.

One alternative for you, that might work right now:

  1. Unleash Edge and the Unleash Proxy do provide an "/all" endpoint, which will deliver all flags.
  2. The SDK do have a client.getAllToggles().

By combining these two you could build your own fallback function like this:

function isEnabled(name, fallback) {
  if(client.getAllToggles().map(t => t.name).includes(name)) {
    return client.isEnabled(name);
  } else {
    return fallback;
  } 
}

@exhuma
Copy link
Author

exhuma commented Aug 29, 2024

I saw the getAllToggles() but that returns an empty list for me. In my mind I discarded that with the thought: "Toggles must be something different from feature-flags so it's normal that I get an empty list and I cannot use it for this use-case".

Another solution could be to have something similar to the "bootstrap" option that - when set - will do an initial "pull" of all the known feature-flags. This could be a fairly simple payload that is sufficient for the isEnabled() to know whether they are known or unknown by the system.

This could be "false" by default to keep the same behaviour as we have now. This could then be used as follows:

const client = new UnleashClient({fetchKnownFlags: true});
client.isEnabled('my-unknown-flag')  // -> undefined
client.isEnabled('my-known-flag')  // -> false

While I'm not a huge fan of abusing undefined like this, it does represent reality and might be of use here. Also, considering that undefined is "falsy" in a boolean context it would work transparently and you can still write the following without risk of tracebacks:

if (client.isEnabled('my-unknown-feature')) { ... }

When using it without the option the result would be:

const client = new UnleashClient({fetchKnownFlags: false});
client.isEnabled('my-unknown-flag')  // -> false
client.isEnabled('my-known-flag')  // -> false

Having the undefined value could be used by the isEnabled function internally to return a fallback value if needed. Or, in the JS-world we could use the nullish coalescing operator to write:

const client = new UnleashClient({fetchKnownFlags: true});
client.isEnabled('my-unknown-flag') ?? false // -> false
client.isEnabled('my-unknown-flag') ?? true // -> true

That way, it would be up to the user to enable that initial "download" of flags or not. If you never need to cover that case, you can leave it off for startup speedup.

It might make sense to periodically refresh that list if that feature is requested from the client. This could be included in the proxy response whenever the list changes.

Additionally/Alternatively, the client can update that list whenever it sees a new feature for the first time. For example, assume that on startup the flags a and b are known and will be loaded. Then the client will always consider checks for c to be undefined. When an admin creates a new feature flag c and toggles it, the client SDK will see an update event with that feature. The client can then safely add c into the "known" flags. It will however not be aware of any deletion of feature flags (but that should be much more rare anyway).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: For later
Development

No branches or pull requests

2 participants