Better error message in ActivatorUtilities

[jorisvergeer]

The current error message gave me no pointers on what was/went wrong when I had an extra parameter to ActivatorUtilities.CreateInstance<SomeClass>(provider, usedParameter, unusedParameter). I experimentally found out that this causes issues.

Adding to the error message that all parameters have to be consumed would have saved me some time, and I hope that this change will save some time for a future developer.

Example that gave me unclear instructions:

        private interface IService
        {
            string Name { get; }
        }

        private class Service : IService
        {
            public string Name { get; set; }
        }

        private class MyClass
        {
            private readonly IService serviceA;
            private readonly IService serviceB;

            public MyClass(IService serviceA, IService serviceB)
            {
                this.serviceA = serviceA;
                this.serviceB = serviceB;
            }

            public void Print()
            {
                Console.WriteLine(serviceA.Name);
                Console.WriteLine(serviceB.Name);
            }
        }

        static void Main(string[] args)
        {
            var services = new ServiceCollection();
            services.AddSingleton<IService>(p => new Service { Name = "Injected" });

            var provider = services.BuildServiceProvider();

            var special = new Service { Name = "Special" };

            // The extra "Unused" string is not consumed by MyClass
            ActivatorUtilities.CreateInstance<MyClass>(provider, special, "Unused");

            // Throws: System.InvalidOperationException: 'A suitable constructor for type 'TestProject.Program+MyClass' could not be located. Ensure the type is concrete and services are registered for all parameters of a public constructor.'
        }

[ghost]

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

---

The current error message gave me no pointers on what was/went wrong when I had an extra parameter to ActivatorUtilities.CreateInstance<SomeClass>(provider, usedParameter, unusedParameter). I experimentally found out that this causes issues.

Adding to the error message that all parameters have to be consumed would have saved me some time, and I hope that this change will save some time for a future developer.

Example that gave me unclear instructions:

        private interface IService
        {
            string Name { get; }
        }

        private class Service : IService
        {
            public string Name { get; set; }
        }

        private class MyClass
        {
            private readonly IService serviceA;
            private readonly IService serviceB;

            public MyClass(IService serviceA, IService serviceB)
            {
                this.serviceA = serviceA;
                this.serviceB = serviceB;
            }

            public void Print()
            {
                Console.WriteLine(serviceA.Name);
                Console.WriteLine(serviceB.Name);
            }
        }

        static void Main(string[] args)
        {
            var services = new ServiceCollection();
            services.AddSingleton<IService>(p => new Service { Name = "Injected" });

            var provider = services.BuildServiceProvider();

            var special = new Service { Name = "Special" };

            // The extra "Unused" string is not consumed by MyClass
            ActivatorUtilities.CreateInstance<MyClass>(provider, special, "Unused");

            // Throws: System.InvalidOperationException: 'A suitable constructor for type 'TestProject.Program+MyClass' could not be located. Ensure the type is concrete and services are registered for all parameters of a public constructor.'
        }

Author:	jorisvergeer
Assignees:	-
Labels:	area-Extensions-DependencyInjection
Milestone:	-

[jorisvergeer]

@eerhardt that is better

[eerhardt]

@jorisvergeer - it looks like there are conflicts with your change. Can you address them?

The ActivatorUtilities.cs file has been moved to src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs.

[jorisvergeer]

@eerhardt Done

[eerhardt]

LGTM

[eerhardt]

Looks like there is now a test failure. Can you fix it up?

[jorisvergeer]

Oh damn.

I didn't think about that the tests would fail because a change in a error message. I kind of assumed that some tests were in a permanent state of failure except for private or release builds. (I've seen some of those setups).

I respect this level of unit tests.

[jorisvergeer]

@eerhardt Lets see how this goes.
Couldn't run the tests locally.

[jorisvergeer]

@eerhardt Shouldn't you have to approve this again after I change code?

[eerhardt]

The tests are still failing. The message appears duplicated here:

runtime/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs

Line 215 in c618d42

string? message = $"A suitable constructor for type '{instanceType}' could not be located. Ensure the type is concrete and services are registered for all parameters of a public constructor.";

Looks like we will need to update that message as well.

[jorisvergeer]

@eerhardt Took a while to get the tests running locally. It's passing now locally.