search

jason-roberts / FeatureToggle

Simple, reliable feature toggles in .NET
http://dontcodetired.com/blog/?tag=/featuretoggle
Apache License 2.0
687 stars 111 forks source link

The key 'FeatureToggle.xxx' was not found in AppSettings #5

Closed laurentkempe closed 10 years ago

laurentkempe commented 11 years ago

Not having the key defined in AppSettings should default it to false, so that you do not expose those information to customers which might look at the file

jason-roberts commented 11 years ago

Hi, thanks for the comment. I disagree. If you are using a toggle that expects a value to be in the settings then it should be there: either on or off. By defaulting to false there is no way to know whether you have made an error in the config or you actually wanted the toggle to be off.

It would however be possible to create some kind of obfuscated toggle value provider that (for example) stored things in the registry in an "encrypted" way.

laurentkempe commented 11 years ago

Hi, I agree when using something like SimpleFeatureToggle, but you could have a new one which would default to false when a value is not defined! This would hurt any existing code and still believe that it is a valid usage case. The registry "encrypted" way it to complex for users to use.

How would have a feature toggle which is available for internal users?

alexsorokoletov commented 7 years ago

On one hand, I agree with Jason and, by the way, big thanks for the framework - well done!

On another, we just had a deployment when binaries were deployed but msdeploy did not update web.config (there is a bug in msdeploy, https://github.com/Microsoft/vsts-tasks/issues/1607).

Because of that we got like hundreds failed requests (with the error from this issue) until we figured out that it's coming from FeatureToggle and mitigated the problem.

I'd be glad to have a way to allow FeatureToggle to default all missing keys to false if I state that explicitly.

What do you think, @jason-roberts?

jason-roberts commented 7 years ago

@alexsorokoletov thanks for the thanks :)

I think this is potentially dangerous, defaulting to false might not always be a safe thing to assume. However I can see that for some uses like your situation you might want to. If implemented this would not be the default setting and you'd have to explicitly enable it at some global level...

Rather than a global setting how would you feel about using a decorator that defaults to false for specific instances of toggles (rather than a global setting)? I guess it would depend how many toggles you had.

The library already has these decorators:

https://github.com/jason-roberts/FeatureToggle/blob/89f79625be588411a34d7a1c8236fa6c8ca217c5/src/FeatureToggle.NetStandard/DefaultToDisabledOnErrorDecorator.cs

https://github.com/jason-roberts/FeatureToggle/blob/89f79625be588411a34d7a1c8236fa6c8ca217c5/src/FeatureToggle.NetStandard/DefaultToEnabledOnErrorDecorator.cs

https://github.com/jason-roberts/FeatureToggle/blob/89f79625be588411a34d7a1c8236fa6c8ca217c5/src/Tests/FeatureToggle.Shared.Tests/FallbackValueDecoratorShould.cs

alexsorokoletov commented 7 years ago

@jason-roberts decorators will do it, I guess. Thank you for quick response!

Even though your docs clearly stay that if something goes wrong FeatureToggle will throw an exception, I think I was fooled by repo tagline "Simple, reliable feature toggle".

Honestly, the tagline is why I chose this project over many other.

update: I've used these decorators, they solve the task just fine. One thing I don't like though in these decorators - exception-based logic :)