search

microsoft / FeatureManagement-Dotnet

Microsoft.FeatureManagement provides standardized APIs for enabling feature flags within applications. Utilize this library to secure a consistent experience when developing applications that use patterns such as beta access, rollout, dark deployments, and more.
MIT License
1.02k stars 111 forks source link

Moves cache busting and handles null #438

Closed rossgrambo closed 3 months ago

rossgrambo commented 3 months ago

Why this PR?

A developer had an issue. After refresh they were getting:

Object reference not set to an instance of an object

at Microsoft.FeatureManagement.FeatureManager.\d__10.MoveNext()

After trying to recreate the issue, I found one way. The steps are as follows:

  1. Call FeatureManager.GetFeatureNamesAsync().
  2. Iterate the first item in the AsyncEnumerable. ( This means, ConfigurationFeatureDefinitionProvider.GetAllFeatureDefinitionsAsync will iterate once, meaning it will grab a list of all IConfigurationSections defined within FeatureManagement, but only iterate the first one )
  3. Remove a flag from the Configuration
  4. Refresh configuration
  5. Call IsEnabledAsync("RemovedFlag") or iterate through a full GetFeatureNamesAsync. This fills the cache with "null".
  6. Continue iterating the first AsyncEnumerable.

In short- the cache gets busted when the refresh happens, but the old AsyncEnumerable is not updated. It expects the IConfigurationSection it saw before iterating to still exist.

Visible Changes

This fix is in two places-

  1. The AsyncEnumerable in ConfigurationFeatureDefinitionProvider will check for null iteration- since iterations can be as slow as the developer wants. If the same situation happens- there might be a null in the cache, but we won't return it.
rossgrambo commented 3 months ago

Updated to use a null check instead of cache busting.