Configuration binding swallows the conversion exception

[jvmlet]

Description

Configuration binding still swallows the conversion exception (#36502 ,#36129 and #36015 are still reproducible with 6.0.0, )

Reproduction Steps

public enum  Ingredients {
A,
B
}

public class  MyModel{
public  Ingredients[] {get;set;}
}

Starting the app with myapp.exe --MyModel:Ingredients:0 A --MyModel:Ingredients:1 C starts up with Ingredients == [A]

Expected behavior

Exception should be thrown when calling iConfiguration.GetSection("MyModel").Bind(new MyModel(),opts => opts.ErrorOnUnknownConfiguration = true))

Actual behavior

application starts with Ingredients == [A]

Regression?

No response

Known Workarounds

No response

Configuration

net5.0 , <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="6.0.0" />

Other information

(

runtime/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs

Line 543 in c0dabb5

{

)

[ghost]

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

Issue Details

---

Description

Configuration binding still swallows the conversion exception (#36502 is still reproducible with 6.0.0 )

Reproduction Steps

public enum  Ingredients {
A,
B
}

public class  MyModel{
public  Ingredients[] {get;set;}
}

Starting the app with myapp.exe --MyModel:Ingredients:0 A --MyModel:Ingredients:1 C starts up with Ingredients == [A]

Expected behavior

Exception should be thrown when calling iConfiguration.GetSection("MyModel").Bind(new MyModel(),opts => opts.ErrorOnUnknownConfiguration = true))

Actual behavior

application starts with Ingredients == [A]

Regression?

No response

Known Workarounds

No response

Configuration

net5.0 , <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="6.0.0" />

Other information

No response

Author:	jvmlet
Assignees:	-
Labels:	area-Extensions-Configuration
Milestone:	-

[tarekgh]

Looks there are some places in the ConfigurationBinder.cs which we swallow the exception. We may consider using BinderOptions.ErrorOnUnknownConfiguration in such places and allow throwing there.

runtime/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs

Line 614 in 8be2105

catch

runtime/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs

Line 647 in 8be2105

catch

runtime/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs

Line 696 in 8be2105

catch

runtime/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs

Line 755 in 8be2105

catch

We may consider the fix in the future releases.

[jvmlet]

Is there NEAREST Future milestone ?
You ship the application documenting all the configuration options and have no chance to validate them at runtime.
No observability at all
Sounds like a blocker issue.
BTW, don't you have static code analysis as build-pipeline step ? Sonar has no issue to catch this violation

[fnajera-rac-de]

I just wasted 4 hours debugging until I arrived at this issue. I agree with @jvmlet that this should be a blocker and at the very minimum a binding option should be provided to fail on children error.

[jvmlet]

@tarekgh , would you please prioritize it ?

[tarekgh]

@jvmlet I marked with the next release milestone. Are you interested in submitting a PR for it?

[SteveDunn]

I'm happy to look at this as I vaguely remember adding ErrorOnUnknownConfiguration. I'm guessing there's no real urgency for this as time is a bit short at the moment.

[SteveDunn]

The comments for ErrorOnUnknownConfiguration say:

/// <summary>
/// When false (the default), no exceptions are thrown when a configuration key is found for which the
/// provided model object does not have an appropriate property which matches the key's name.
/// When true, an <see cref="System.InvalidOperationException"/> is thrown with a description
/// of the missing properties.
/// </summary>

So, technically, in this case, the key exists, and the config exists, so 🙈

Of course, that config option implies that an error should be thrown, not just on missing properties in config, but also invalid properties in config.

By making ErrorOnUnknownConfiguration dual-purpose might now introduce unexpected breaking changes.

Maybe we need another property named ErrorOnInvalidConfiguration.

What do you think? Would this require another API Proposal?

[tarekgh]

I don't think we need to complicate the options by adding one more property like that. We can use ErrorOnUnknownConfiguration and clarify the documentation. Yes, it will be a breaking change, but I don't think anyone using ErrorOnUnknownConfiguration was not trying to catch all thrown exception when loading the configuration. Let me know if you think otherwise.

[tarekgh]

This is already done by the PR #77708