Skip to content

Fix/issue 441#471

Merged
nblumhardt merged 6 commits intoserilog:devfrom
gyurebalint:fix/issue-441
May 2, 2026
Merged

Fix/issue 441#471
nblumhardt merged 6 commits intoserilog:devfrom
gyurebalint:fix/issue-441

Conversation

@gyurebalint
Copy link
Copy Markdown
Contributor

Fixes #441

Problem

When a configuration method has a params array parameter (e.g. params string[] ids) and the Args section is omitted entirely from config, the library fails to invoke the method at startup with:
System.ArgumentException: Object of type 'System.DBNull' cannot be converted to type 'System.String[]'.

This happens because:

  1. HasImplicitValueWhenNotSpecified does not recognise params parameters as optional — so SelectConfigurationMethod rejects the method entirely when no args are supplied
  2. Even if the method were selected, GetImplicitValueForNotSpecifiedKey falls through to return parameter.DefaultValue which is DBNull for params parameters on .NET 10 — causing a crash at invocation

Changes

ConfigurationReader.cs

  • Added ParamArrayAttribute check to HasImplicitValueWhenNotSpecified so params parameters are treated as optional during method selection
  • Added ParamArrayAttribute check to GetImplicitValueForNotSpecifiedKey to return an empty array of the correct element type instead of DBNull
  • Made GetImplicitValueForNotSpecifiedKey
    internal to allow direct unit testing

ConfigurationReaderTests.cs

  • Added test asserting that a method with a params string[] parameter is selected when no args are supplied
  • Added test asserting that the implicit value for a params string[] parameter is an empty array, not DBNull

Root cause

In .NET 10, params parameters report HasDefaultValue = true but DefaultValue is DBNull — not a valid value for an array type. The fix intercepts params parameters before falling through to return parameter.DefaultValue.

@gyurebalint
Copy link
Copy Markdown
Contributor Author

Hi everyone!
Note: I changed GetImplicitValueForNotSpecifiedKey to internal to allow for isolated unit testing of the params logic.
If you'd prefer to keep it private and have this tested strictly through the public API, let me know and I'll adjust the tests.
Any feedback is appreciated.

Copy link
Copy Markdown
Member

@nblumhardt nblumhardt left a comment

Choose a reason for hiding this comment

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

Thanks for sending this!

params is now supported on spans, enumerables, and other collections:

https://devblogs.microsoft.com/dotnet/csharp13-calling-methods-is-easier-and-faster/#params-collections

It would be great to extend test coverage to make sure these cases work, or at least degrade gracefully.

$"This is not supported when only a `IConfigSection` has been provided. (method '{methodToInvoke}')");
}

if (parameter.IsDefined(typeof(ParamArrayAttribute), false))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If this is modified to:

if (parameter.IsDefined(typeof(ParamArrayAttribute), false) &&
    parameter.ParameterType.GetElementType() is {} elementType)

then elementType can be used in the CreateInstance call without !, and we'll also not fail in (as yet unknown) cases where ParamArrayAttribute is present but the parameter type isn't an array.

Copy link
Copy Markdown
Contributor Author

@gyurebalint gyurebalint May 1, 2026

Choose a reason for hiding this comment

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

I’ve updated the PR!

This new params support begs the question, should this library handle these cases, and if so how?

Ideas:

  • IEnumerable could just return an array, closest type to it, and further handling can happen on the client side
  • List and other generics I think can be handled generically, by returning an instance of the collection with T type
  • ReadOnlySpan is a whole different beast.. and could be a big change since now we are returning object? in GetImplicitValueForNotSpecifiedKey so this method won't be able to handle it.

@nblumhardt
Copy link
Copy Markdown
Member

LGTM, thanks. It would be nice to extend support to different params types - though things might get tricky in the Span case. Extending to the generic enumerable/list types seems like a win, though.

@nblumhardt nblumhardt merged commit 427ec51 into serilog:dev May 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

params arguments should be regarded as optional

2 participants