Skip to content

Commit

Permalink
Added option to throw when attempting to evaluate a missing feature. (#…
Browse files Browse the repository at this point in the history
…140)

* Added option to throw for missing features.

* Add default value documentation for feature management options.
  • Loading branch information
jimmyca15 committed Sep 1, 2021
1 parent e531863 commit 65a4551
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 1 deletion.
7 changes: 6 additions & 1 deletion src/Microsoft.FeatureManagement/FeatureManagementError.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ public enum FeatureManagementError
/// <summary>
/// A feature filter configured for the feature being evaluated is an ambiguous reference to multiple registered feature filters.
/// </summary>
AmbiguousFeatureFilter
AmbiguousFeatureFilter,

/// <summary>
/// A feature that was requested for evaluation was not found.
/// </summary>
MissingFeature
}
}
8 changes: 8 additions & 0 deletions src/Microsoft.FeatureManagement/FeatureManagementOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,15 @@ public class FeatureManagementOptions
/// <summary>
/// Controls the behavior of feature evaluation when dependent feature filters are missing.
/// If missing feature filters are not ignored an exception will be thrown when attempting to evaluate a feature that depends on a missing feature filter.
/// The default value is false.
/// </summary>
public bool IgnoreMissingFeatureFilters { get; set; }

/// <summary>
/// Controls the behavior of feature evaluation when the target feature is missing.
/// If missing features are not ignored an exception will be thrown when attempting to evaluate them.
/// The default value is true.
/// </summary>
public bool IgnoreMissingFeatures { get; set; } = true;
}
}
13 changes: 13 additions & 0 deletions src/Microsoft.FeatureManagement/FeatureManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,19 @@ private async Task<bool> IsEnabledAsync<TContext>(string feature, TContext appCo
}
}
}
else
{
string errorMessage = $"The feature declaration for the feature '{feature}' was not found.";

if (!_options.IgnoreMissingFeatures)
{
throw new FeatureManagementException(FeatureManagementError.MissingFeatureFilter, errorMessage);
}
else
{
_logger.LogWarning(errorMessage);
}
}

foreach (ISessionManager sessionManager in _sessionManagers)
{
Expand Down
25 changes: 25 additions & 0 deletions tests/Tests.FeatureManagement/FeatureManagement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,31 @@ public async Task SwallowsExceptionForMissingFeatureFilter()
Assert.False(isEnabled);
}

[Fact]
public async Task ThrowsForMissingFeatures()
{
IConfiguration config = new ConfigurationBuilder().AddJsonFile("appsettings.json").Build();

var services = new ServiceCollection();

services
.Configure<FeatureManagementOptions>(options =>
{
options.IgnoreMissingFeatures = false;
});

services
.AddSingleton(config)
.AddFeatureManagement();

ServiceProvider serviceProvider = services.BuildServiceProvider();

IFeatureManager featureManager = serviceProvider.GetRequiredService<IFeatureManager>();

FeatureManagementException fme = await Assert.ThrowsAsync<FeatureManagementException>(() =>
featureManager.IsEnabledAsync("NonExistentFeature"));
}

[Fact]
public async Task CustomFeatureDefinitionProvider()
{
Expand Down

2 comments on commit 65a4551

@eldola-norsea
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warning log message is unfortunate for our use case. I have added features that are only meant to be used in development, so they are by default disabled - and they are not defined in production settings. The warning log messages in production are thus producing unwanted noise.

@rossgrambo
Copy link
Contributor

@rossgrambo rossgrambo commented on 65a4551 Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @eldola-norsea

I'm curious about your use case. Could you give a short example of what your schema looks like in both places?

Something like:

Development

  FlagA: false,
  FlagB: false

Production

  FlagC: {
    ...
  },
  FlagD: {
    ...
  }

Or if using the provider, clarify what the separation between development and production are.

Please sign in to comment.