Repository navigation
Configuration binding swallows the conversion exception #73915
Description
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
(
)
Activity
tarekgh commentedon Aug 15, 2022
Looks there are some places in the ConfigurationBinder.cs which we swallow the exception. We may consider using
BinderOptions.ErrorOnUnknownConfigurationin such places and allow throwing there.runtime/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs
Line 614 in 8be2105
runtime/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs
Line 647 in 8be2105
runtime/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs
Line 696 in 8be2105
runtime/src/libraries/Microsoft.Extensions.Configuration.Binder/src/ConfigurationBinder.cs
Line 755 in 8be2105
We may consider the fix in the future releases.
jvmlet commentedon Aug 16, 2022
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 commentedon Oct 5, 2022
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 commentedon Oct 5, 2022
@tarekgh , would you please prioritize it ?
tarekgh commentedon Oct 5, 2022
@jvmlet I marked with the next release milestone. Are you interested in submitting a PR for it?
SteveDunn commentedon Oct 31, 2022
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 commentedon Oct 31, 2022
The comments for
ErrorOnUnknownConfigurationsay: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
ErrorOnUnknownConfigurationdual-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 commentedon Nov 1, 2022
I don't think we need to complicate the options by adding one more property like that. We can use
ErrorOnUnknownConfigurationand clarify the documentation. Yes, it will be a breaking change, but I don't think anyone usingErrorOnUnknownConfigurationwas not trying to catch all thrown exception when loading the configuration. Let me know if you think otherwise.tarekgh commentedon Nov 8, 2022
This is already done by the PR #77708