Logical problems related to constructor argument resolution

[rastrac-joel]

There's some code in ConstructorInstance.cs:

        private void buildOutConstructorArguments(ServiceGraph services)
        {
            Arguments = Constructor.GetParameters()
                .Select(x => determineArgument(services, x))
                .Where(x => x.Instance != null).ToArray();

The Where clause will never filter out any values because CtorArg prevents any .Instance from being null:

        public CtorArg(ParameterInfo parameter, Instance instance)
        {
            try
            {
                Parameter = parameter ?? throw new ArgumentNullException(nameof(parameter));
                Instance = instance ?? throw new ArgumentNullException(nameof(instance));
                // ...
            }
            catch (Exception e)
            {
                throw new InvalidOperationException(
                    $"Cannot create a Constructor Argument for {parameter.Name} of {instance}", e);

findInstanceForConstructorParameter appears to be intended to never return null, however, ServiceGraph.FindInstance will return null under some circumstances and this is accounted for on the parameter.DefaultValue == null code branch but not outside of parameter.IsOptional:

            // Abridged
            if (parameter.IsOptional)
                if (parameter.DefaultValue == null)
                    return services.FindInstance(parameter) ?? new NullInstance(dependencyType);

            return services.FindInstance(parameter);

ServiceGraph.FindInstance will call ServiceFamily.InstanceFor which returns null in the case where there is no match for the name in the Instances collection and there are no Instance values in the collection that share the name.

Additionally, ServiceGraph.FindDefault will return null in the case where the type matches the Type.IsSimple extension method or the Instance returned is null or the Instance.Default field is null. I'm not sure from the code if it can return a null Instance, but it seems from the usage of LastOrDefault that Instance.Default could be null.

All of these cases will cause an exception to be thrown from CtorArg and it contains no context information that allows the user to figure out where or even what the problem is. All that's known is the constructor argument name and since Instance is attempting to be set to null, there's no type information. The constructor argument name could be shared among tens or hundreds of classes. In the exception I'm getting, the constructor argument isn't a defaulted argument in any of the applicable classes either (and I gather from other issues that this could be a problem).

The context here is that I'm attempting to update a library that uses StructureMap to using Lamar (specifically for ASP.NET Core on .NET 6 if that makes a difference) and it's not going well and the library does not specify enough information here to understand the problems. I'm sure I'm doing something incorrectly - or maybe the code was doing something that StructureMap was permissive of but Lamar is not. In any case, what it is that I'm doing incorrectly and how to go about fixing it is a different issue. I tried searching for the error messages on multiple search engines and they came up with very little.

Regardless, the invariants that are assumed by the code are sending mixed messages. I don't know enough about the library to suggest any particular improvements, only to raise visibility that this is causing issues and the code itself seems internally confused.

[rastrac-joel]

Seems as though the registrations were binding constructor arguments to a different name than specified by the constructor. I guess StructureMap must have automatically associated them if there was only one argument of the argument type. I don't think this is mentioned in the related documentation I had read.

Still, attempting to bind to an argument that isn't there seems like a common error that should be accounted for. It's obvious enough when writing new code, but when trying to convert and you have hundreds or thousands of lines of registration code, it's a lot less clear which ones are the problems. I wasn't aware that the constructor argument was the string I was looking at in the exception message since the exception was being thrown in internal code. The name happened to be one that a library might have.

[jeremydmiller]

@rastrac-joel Sorry, but I don't understand from this explanation what you're not able to do. Can you put some kind of concrete code in here that demonstrates your problem? For IoC usage, and I know this doesn't help, but I strongly recommend against using types with optional arguments in constructor functions being used with Lamar because of exactly this kind of potential confusion.

[jeremydmiller]

There's just nothing actionable here, I'm closing this unless someone can add more concrete information about what the exact problem is.

[rastrac-joel]

I found that the problem was StructureMap was permissive of the case where you might have something like

class SomeService : ISomeService
// ...
    public SomeService(string configurationString)
// ...

class SomeRegistry : Registry
// ...
    For<ISomeService>().Use<SomeService>().Ctor<string>("connectionString").Is(connectionString).Singleton();
// ...

This worked in StructureMap (probably shouldn't have) and Lamar is not permissive of it which is perfectly fine behavior.

The specific problem that I had is that the code in Lamar indicates no helpful context with its exception for the null that goes along with failing to register a proper constructor argument. All that ends up in the exception is the parameter name of the constructor argument. In the case of the code I was updating to Lamar, because this worked in StructureMap, there were quite a few equivalents of ISomeService implementations that had the same kind of registration-parameter name disconnect, so the only way I could find them "easily" was taking the Lamar projects and putting them into my solution to be able to walk up the stack to the point where the useful context information was present. The exception message only includes what the parameter name is, not what other things might be registered to the constructor (or the type). ISomeService implementations were in packages so there was no way for me to easily determine what was affected once I had figured out what was happening and it was very confusing up until the point where I did find out what was happening. If the type had been present in the exception message, it would have been fairly clear upon investigating the specific type.

Converting from StructureMap to Lamar means that there might be a lot of types that fall under this criteria, so at a minimum, the exception carrying more information (in particular, the concrete type) is preferable. This is fairly concrete and actionable. I would think upgrading from StructureMap to Lamar would be something that would be desired or expected from consumers of StructureMap and it would still benefit development using Lamar even if StructureMap is removed from the equation.

I was investigating Lamar's code to attempt to discern what might be the specific problem and found that it's written in a way seemingly confused about its own invariants. I think it would be of benefit to review that as well but since I don't really understand what went into the code being the way it is, there's not much I can provide there except a summary of the things I found.

[jeremydmiller]

Okay, now I got you, and that's a happy, unintentional "feature" of StructureMap. I even know the loosey goosey code in SM that did the inline matching like that. The "easy" way going forward IMO is to not use primitive/simple types in the container. Even in the SM days, I've recommended using strong typed "Setting" or now .NET "Options" classes instead of trying to rely on named constructor values for connection strings, file paths, etc.. That's my 2 cents.

If you can code a repo, I can do something to attempt a better exception message. TBH, I really just want to strongly recommend not using simple types as dependencies period. Or use a lambda registration that pulls the connection string out of IConfiguration as an alternative approach.