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

Misconfigured Flags Should Error #68

Closed
jason-roberts opened this issue May 13, 2020 · 6 comments
Closed

Misconfigured Flags Should Error #68

jason-roberts opened this issue May 13, 2020 · 6 comments

Comments

@jason-roberts
Copy link
Contributor

I think that if a flag is used and that flag is not configured then the app should error and not just default the flag to false.

For example, if a configured flag is for some reason omitted in a production config (or mis-spelled) then the app will continue to run without error and the fault may not be noticed.

If a flag is being used programmability then this could cause data issues.

If a flag is being used to control UI elements such as some legal wording that needs to be shown, then by defaulting to false the company could be in breach of laws.

@zhenlan
Copy link
Member

zhenlan commented May 23, 2020

A potential solution is to allow customization of the behavior when a flag is not found. By default, the flag is considered false, but a user can change the default to true or throw.

@josephwoodward
Copy link
Contributor

A potential solution is to allow customization of the behavior when a flag is not found. By default, the flag is considered false, but a user can change the default to true or throw.

I agree, this seems like a better solution.

@jimmyca15
Copy link
Member

I believe we should add a new error type to FeatureManagementError. The type can be MissingFeature. Once we have the new error, we can throw a FeatureManagementException when a call for a missing feature occurs. We can then offer a switch in FeatureManagementOptions to control the new behavior.

If the behavior is on by default then it is a breaking change so it would need to be done in a major version bump.

@josephwoodward
Copy link
Contributor

@jimmyca15 I have a draft PR here for this change, interested in any feedback before I continue on this approach.

@maplemale
Copy link

maplemale commented Aug 30, 2021

Any chance this can be bumped / moved forward? Looks there was a PR with comments and it is now stale with conflicts?

@jimmyca15
Copy link
Member

Enabled in release 2.4.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants