Skip to content

Add BinderOption to allow callers to specify that configuration binding should throw an exception upon any failure. #36015

Closed
@BlacKCaT27

Description

@BlacKCaT27

UPDATED PROPOSAL

Background and Motivation

The configuration binding system provided by Microsoft.Extensions.Configuration currently does not throw any kind of exception should there be any issue binding configuration data to a model object. Scenarios such as a key missing from a configuration provider, or a model object not being updated to provide access to a new configuration key can often result in difficult to diagnose run-time errors, some of which might not be caught until a deployment, as one of the few differences between testing and production environments are configuration values.

There would be value in providing users of the configuration system a mechanism to make model binding in either direction an exception-throwing event to make it more immediately apparent when issues arise due to configuration mismatches.

Proposed API

Create a new exception Microsoft.Extensions.Configuration.ConfigurationException. As much as possible, this exception should be designed to include as much information as possible based on the context in which it is throw. Given the proposals below, the exception should, at a minimum, always include the name of the configuration key and (when possible)/or (when not) the model property that was being bound at the time the exception was thrown.

Add two new properties to the Microsoft.Extensions.Configuration.BinderOptions class:

public class BinderOptions
    {
        /// <summary>
        /// When false (the default), the binder will only attempt to set public properties.
        /// If true, the binder will attempt to set all non read-only properties.
        /// </summary>
        public bool BindNonPublicProperties { get; set; }

+       /// <summary>
+       /// If set to true, the configuration system would throw a `ConfigurationException` if, during configuration binding, a 
+       /// configuration key is found for which the provided model object does not have an appropriate property which matches
+       /// the keys name. Note: appropriate property here refers to a property that is eligible to be bound to (for example, a
+       /// private property would not be considered an `eligible` target property unless 
+       /// </summary>
+       public bool ErrorOnPropertyMissing { get; set; }

+       /// <summary>
+       /// If set to true, the configuration system would throw a `ConfigurationException` if, during configuration binding, a 
+       /// property is found on a target model object which does not have a corresponding configuration key provided by any 
+       /// configuration providers. Default value is false.
+       /// </summary>
+       public bool ErrorOnKeyMissing { get; set; }
    }

Usage Examples

Below is an example when BinderOptions is being used alongside the Microsoft.Extensions.Options package. The usage would be the same for any other location where BinderOptions can be provided as an argument.

            services.AddOptions<RequestHandlingOptions>().Bind(Configuration.GetSection("RequestHandlingOptions"), options =>
            {
                options.BindNonPublicProperties = true;
                options.ErrorOnKeyMissing = true;
                options.ErrorOnPropertyMissing = true;
            });

Alternative Designs

I had considered including a 3rd flag as a new property: BinderOptions.ErrorOnConfigurationMismatch which would perhaps throw an exception in both cases proposed above, and/or additionally in other cases (such as model property count/configuration key count mismatch, which may or may not imply the above scenarios in some cases). Ultimately though I felt a simpler approach was best to start.

Or instead of adding the two properties above, add just one properties that controls throwing on all mismatches instead.

public class BinderOptions
    {
        /// <summary>
        /// When false (the default), the binder will only attempt to set public properties.
        /// If true, the binder will attempt to set all non read-only properties.
        /// </summary>
        public bool BindNonPublicProperties { get; set; }

+       /// <summary>
+       /// If set to true, the configuration system would throw a `ConfigurationException` if, during configuration binding, a 
+       /// configuration key is found for which the provided model object does not have an appropriate property which matches
+       /// the keys name; or if a configuration property is found without a matching key. 
+       /// </summary>
+       public bool ErrorOnConfigurationMismatch { get; set; }
}

Risks

As these flags would be disabled by default, the existing behavior of the configuration binding process would not change. However, developers should be encouraged to thoroughly test their configuration management systems when enabling these flags, given the environment-dependent nature of configuration management and the potential impact of the change should there be any issues with a systems configuration.

ORIGINAL POST

Is your feature request related to a problem? Please describe.

Currently, when using configuration binding, in the event that a property does not get bound (either because there's a missing entry in the configuration file or a missing property on the class being bound to), the binder simply defaults to null for that property. This results in run-time null exceptions if there is a configuration issue.

Describe the solution you'd like

I propose we add an optional setting in the new BinderOptions class which allow callers to specify that in the event of a failure to properly bind every configuration option, to throw an exception rather than passively set property values to null.

An optional extension of this idea would be to also allow callers to specify custom validators for specific configuration values. For example, being able to annotate the POCO that a configuration file is being bound to such that something like a Connection String could have a custom validator called on it to make sure all the required arguments are contained in the string, the database is reachable, etc.

Describe alternatives you've considered

The alternative is to use something like an IStartupFilter to manually check all your configuration parameters validity during application start-up. This is tedious and error-prone, and results in application level code having to validate behaviors provided by framework level code, mixing concerns.

Additional context

I believe this feature will add a lot of value at relatively low cost. It's much easier to find misconfiguration errors if start-up fails because of app misconfiguration, rather than having to track down unexpected null exceptions thrown from some piece of code using configuration values.

It looks like there's been some work/discussion on this front in other forms (issue dotnet/extensions#459 and issue dotnet/extensions#763), but those are more focused on the larger problems of IOption and custom validators. Perhaps this issue can focus solely on establishing a starting point via making Bind() throw on error an option.

I'd be more than happy to put up an initial PR to explore this further.

Metadata

Metadata

Assignees

Labels

api-approvedAPI was approved in API review, it can be implementedarea-Extensions-Configurationhelp wanted[up-for-grabs] Good issue for external contributors

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions