[API Proposal]: ConfigurationBinder.GetRequiredValue

[MattKotsenas]

Background

ConfigurationBinder has a method GetValue<T>, which is a simple way of getting a single value from configuration and converting it to the specified type (docs). The API optionally accepts a default value if the configuration key cannot be found. If the key is not found and no default is specified, default(T) is returned. If the value is found but cannot be converted to the target type, an exception is thrown.

Motivation

In some cases, silently returning default(T) can lead to subtle bugs. For example, consider an example app that requires a timeout value, which should always be specified in appsettings.json, and is retrieved like this var timeout = configuration.GetValue<TimeSpan>("Timeout"). In our example the config value is missing. Since no default is specified, the default TimeSpan is returned, which is... 00:00:00! As a result, all operations time out instantly rather that what we might expect, which is some type of exception that the configuration key wasn't found.

.NET 6 introduced a GetRequiredSection extension that performs similar validation for configuration sections, so I thought it may be appropriate to extend that convenience to the single value case as well.

Note that the proposed APIs would be on the ConfigurationBinder class in Microsoft.Extensions.Configuration.Binder and not ConfigurationExtensions in Microsoft.Extensions.Configuration.Abstractions since GetValue is exposed through Binder and isn't part of the Abstractions layer.

API Proposal

namespace Microsoft.Extensions.Configuration
{
    public partial static class ConfigurationBinder
    {
        public static T GetRequiredValue<T>(this IConfiguration configuration, string key);
        public static object GetRequiredValue(this IConfiguration configuration, Type type, string key);
    }
}

I propose that the exception if the configuration key isn't found is an InvalidOperationException, since that's what GetRequiredSection throws, but am open to other suggestions.

API Usage

Configuration:

{
    "MyTimeout": "00:00:10"
}

Comparison of usage (generic):

var endpoint = configuration.GetRequiredValue<TimeSpan>("MyTimeout"); // returns '00:00:10'

var endpoint = configuration.GetRequiredValue<TimeSpan>("MyMispelledTimeout"); // throws exception
var endpoint = configuration.GetValue<TimeSpan>("MyMispelledTimeout"); // returns '00:00:00'

Comparison of usage (non-generic):

var endpoint = configuration.GetRequiredValue(typeof(TimeSpan), "MyTimeout"); // returns '00:00:10'

var endpoint = configuration.GetRequiredValue(typeof(TimeSpan), "MyMispelledTimeout"); // throws exception
var endpoint = configuration.GetValue(typeof(TimeSpan), "MyMispelledTimeout"); // returns 'null'

Alternative Designs

The non-generic case can be simulated with a one-liner like this:

var timeout = configuration.GetValue(typeof(TimeSpan), "MyTimeout") ?? throw new InvalidOperationException();

which could then be made generic with another cast, but seems a bit unwieldy, especially if used in multiple places.

The generic version is more important in my opinion, since it's the case that can introduce confusion by coercing a missing value to the default (especially for value types). The non-generic version is proposed mostly for symmetry and reflection scenarios, and thus if it's decided that the non-generic version is not wanted, I wouldn't be opposed to dropping it.

Risks

Low as far as I can see. It's a new, opt-in API that follows an existing naming convention, and that increases type safety, so the likelihood of misuse or abuse seems low.

[ghost]

Tagging subscribers to this area: @dotnet/area-extensions-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

ConfigurationBinder has a method GetValue<T>, which is a simple way of getting a single value from configuration and converting it to the specified type (docs). The API optionally accepts a default value if the configuration key cannot be found. If the key is not found and no default is specified, default(T) is returned.

In some cases, silently returning default(T) can lead to subtle bugs. For example, consider an app that requires a timeout value, which should always be specified in appsettings.json, and is retrieved like this var timeout = configuration.GetValue<TimeSpan>("Timeout"). In this example the config value is missing. Since no default is specified, the default TimeSpan is returned, which is... 00:00:00. As a result, all operations time out instantly rather that what one might expect, which is some type of exception that the configuration key wasn't found.

.NET 6 introduced a GetRequiredSection extension that performs similar validation for configuration sections, so I thought it may be appropriate to extend that convenience to the single value case as well.

API Proposal

namespace Microsoft.Extensions.Configuration
{
    public partial static class ConfigurationExtensions
    {
        public static T GetRequiredValue<T>(this IConfiguration configuration, string key);
        public static object GetRequiredValue(this IConfiguration configuration, Type type, string key);
    }
}

I propose that the exception if the configuration key isn't found by an InvalidOperationException, since that's what GetRequiredSection throws, but am open to other suggestions.

API Usage

Configuration:

{
    "MyEndpoint": "http://localhost"
}

Comparison of usage (generic):

var endpoint = configuration.GetRequiredValue<string>("MyEndpoint"); // returns 'http://localhost'
var endpoint = configuration.GetRequiredValue<string>("MyMispelledEndpoint"); // throws exception
var endpoint = configuration.GetValue<string>("MyMispelledEndpoint"); // returns 'null'

Comparison of usage (non-generic):

var endpoint = configuration.GetRequiredValue(typeof(string), "MyEndpoint"); // returns 'http://localhost'
var endpoint = configuration.GetRequiredValue(typeof(string), "MyMispelledEndpoint"); // throws exception
var endpoint = configuration.GetValue(typeof(string), "MyMispelledEndpoint"); // returns 'null'

Alternative Designs

The non-generic case can be simulated with a one-liner like this:

var timeout = configuration.GetValue(typeof(string), "MyEndpoint") ?? throw new InvalidOperationException();

which could then be made generic with another cast, but that seems a bit unwieldy, especially if used in multiple places.

The generic version is more important in my opinion, since it's the case that can introduce confusion by coercing a missing value to the default (especially for value types). The non-generic version is proposed mostly for symmetry and reflection scenarios, and thus if it's decided that the non-generic version is not wanted, I wouldn't be opposed to dropping it.

Risks

Low as far as I can see. It's a new, opt-in API that follows an existing naming convention, and that increases type safety, so the likelihood of misuse or abuse seems low.

Author:	MattKotsenas
Assignees:	-
Labels:	api-suggestion, untriaged, area-Extensions-Configuration
Milestone:	-

[DrkWzrd]

What should the function return or do if key is found but the value cannot be parsed/bound to the introduced type?

[MattKotsenas]

What should the function return or do if key is found but the value cannot be parsed/bound to the introduced type?

Today GetValue<T> throws an InvalidOperationException with a message of the format Failed to convert configuration value at '{key}' to type '{type}'.

So I would suggest doing the same. I'll update the proposal to make that explicit.

[MattKotsenas]

After looking at how I would implement this code a bit more, it seems that the Abstractions package doesn't expose GetValue, it's exposed from Binder, so I've updated the proposal to have the new method on ConfigurationBinder.

Also here's a sample implementation on my fork: https://github.com/dotnet/runtime/compare/main...MattKotsenas:feature/get-required-value-69163?expand=0

[MattKotsenas]

Hi there! I believe the proposal is ready for feedback, and the sample implementation has been updated with tests. Any suggestions on how to proceed is greatly appreciated. Thanks!

[Eli-Black-Work]

Would it make sense to go ahead and open a PR, so the .NET team can review it that way?

[Eli-Black-Work]

(We'd also be interested in a feature like this 🙂)

[teemka]

In .NET 7 nullable reference types have been enabled in ConfigurationBinder so GetValue method is now marked that it can return null. After migration to .NET 7 I get a warning almost everywhere I use GetValue<T> because of the possible null. The solution to this is to add ! null forgiving operator everywhere I use GetValue but it's not an elegant solution.

This change should've been shipped with .NET 7 :(

[MattKotsenas]

@maryamariyan , @terrajobst , this proposal seems to have stalled out; is there anything we can do to get an approval / rejection and move forward?

If you have any questions, please let me know. Thanks!

[terrajobst]

@maryamariyan didn't we just add validation to configuration? Could this be achieved more generally with that?

[tarekgh]

@MattKotsenas how blocking this scenario for you? This is not a high priority for us which we have marked for the future, and we can consider it later.

[Eli-Black-Work]

@MattKotsenas I remember you had a fork open with a sample implementation. Would it make sense to open a PR for the .NET team to review?

[tarekgh]

We cannot accept a PR before design review any proposal. The issue is not about the implementation. I need to figure out the priority for that request to allocate time for that to investigate and analyze the proposal.

[Eli-Black-Work]

Ah, got it 🙂

[sliekens]

What is the currently the recommended usage pattern for required configuration? e.g. for HttpClient base addresses.

{
    "CoolService": {
        "BaseAddress": "https://cool-stuff/"
    }
}

What is the pattern for turning this into a not-null System.Uri, or throw when the config is missing (or invalid format)?

// a
Uri a = new Uri(Configuration.GetRequiredSection("CoolService:BaseAddress").Value ?? throw new Exception());

// b
Uri b = Configuration.GetRequiredSection("CoolService").GetValue<Uri>("BaseAddress") ?? throw new Exception();

// c
Uri c = new Uri(Configuration.GetRequiredSection("CoolService")["BaseAddress"] ?? throw new Exception());

// d
Uri d = new Uri(Configuration.GetSection("CoolService")["BaseAddress"] ?? throw new Exception());

// e
Uri e = Configuration.GetSection("CoolService").GetValue<Uri>("BaseAddress") ?? throw new Exception();

// f
Uri f = new Uri(Configuration.GetSection("CoolService:BaseAddress").Value ?? throw new Exception());

// g
Uri g = new Uri(Configuration["CoolService:BaseAddress"] ?? throw new Exception());

---

I think the proposed design looks good, it would simplify my code.

Uri future = Configuration.GetRequiredValue<Uri>("CoolService:BaseAddress");

[julealgon]

I think this is a good suggestion that aligns well with other APIs such as IServiceProvider.GetService vs IServiceProvider.GetRequiredService.

It is particularly annoying how it plays out with NRT and the added runtime protection against typos and misconfiguration is nice.

The only aspect I find annoying with this is the indexer access that IConfiguration exposes, which cannot have an equivalent "required" version:

public void Method(IConfiguration configuration)
{
    var option = configuration["MySection:MyOption"]; // returns `null` if option is not present
    ...
}

Would it be a best practice then, moving forward, to never use the indexer access for "normal" mandatory options and prefer GetRequiredValue<string> instead?

[Dreamescaper]

I would suggest to add a non-generic overload, returning string:

public static string GetRequiredValue(this IConfiguration configuration, string key);