Error message for missing service registration is misleading when using AddHttpClient<>

[andrewjsaid]

Description

This is only a misleading error message. If a service is not registered it usually gives the error message:

InvalidOperationException: No service for type 'MyService' has been registered.

However if we register a typed HttpClient for this service then the error message is misleading:

InvalidOperationException: A suitable constructor for type 'MyService' could not be located. Ensure the type is concrete and all parameters of a public constructor are either registered as services or passed as arguments. Also ensure no extraneous arguments are provided.

It's easy to spot the problem in this small example but I spent a while trying to understand the problem in a larger more complex solution, where to obfuscate matters further services were being resolved dynamically (I know, I know).

Reproduction Steps

Run the app below and observe the misleading error message. Then comment out the HttpClient registration and run again and observe the clear error message.

using Microsoft.AspNetCore.Mvc;

var builder = WebApplication.CreateBuilder(args);

// Adding this next line changes the error message to be unclear.
builder.Services.AddHttpClient<MyService>();

var app = builder.Build();

app.MapGet("/hello", ([FromServices] MyService service) => service.Hello());

app.Run();

public class MyService(IHttpClientFactory httpClientFactory)
{
    public string Hello()
    {
        return "World";
    }
}

Expected behavior

AddHttpClient should not change error message of missing registration.

Actual behavior

The more helpful error message should always be shown.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

[julealgon]

However if we register a typed HttpClient for this service then the error message is misleading:

InvalidOperationException: A suitable constructor for type 'MyService' could not be located. Ensure the type is concrete and all parameters of a public constructor are either registered as services or passed as arguments. Also ensure no extraneous arguments are provided.

I don't agree that the error is misleading here. The overloads of AddHttpClient that take interface/implementation types as parameters actually perform a registration for that interface/implementation behind the scenes. Since you are calling AddHttpClient<MyService>, it registers the concrete MyService in the container, then later try to resolve it from there.

If I take your example code as-is and run it, it works just fine: no errors.

What must be happening in your real scenario is that MyService has additional constructor arguments that are not registered in the container, thus generating that error message.

Scratch that.. it does error out when I run your endpoint, and I see your point now.

What is clearly happening here is a call to ActivatorUtilities.CreateInstance is passing in the HttpClient object for your MyService type, but since you don't declare that as a constructor argument, it fails because the binding is required when using activator utilities that way.

It would result in the same error if you did this:

ActivatorUtilities.CreateInstance<MyService>(serviceProvider, new HttpClient());

So in a sense... the message is as expected, you are just misusing the system here. If you want a typed client, you should not be injecting IHttpClientFactory but the HttpClient directly.

[andrewjsaid]

Thank you for the explanation about what happens under the hood. I see that in fact if I inject HttpClient it does indeed work. I was just using httpClientFactory.CreateClient(nameof(MyService)) as I thought it was a valid alternative choice.

I still think the error message could be improved.

[julealgon]

@andrewjsaid

I still think the error message could be improved.

Yeah... to be fair, I don't disagree. I know how this was implemented as I've done some similar things myself before, but I wouldn't expect others to know or even be aware of the ActivatorUtilities class or its behavior necessarily.

I wonder how the team could improve the message in this case however, since they don't have a clean way to check for the issue before it throws AFAIK. Perhaps an analyzer could be a good middle ground, as it should be able to see that the way you are registering and the way you are consuming are incompatible.

To be honest, this whole AddHttpClient<T> set of overloads is a big hack for a missing feature of MEDI, which is its inability to allow for "dependent registrations".

Unfortunately (IMHO), Microsoft decided to create a custom solution specifically for the HttpClient scenario instead of just fixing the underlying limitation and opening the API up for consumers to be able to register these dependencies in a more general way.

With a proper "dependent services" implementation it would of course be much easier to add more specific error messages since the entire API would be more intentional.

[dotnet-policy-service[bot]]

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

[dotnet-policy-service[bot]]

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

[CarnaViire]

Triage: The error message indeed is confusing; and it would be even better if the check could be done in advance during the registration step, not already at the runtime.

While it should be relatively straightforward to do the check, it's just a "nice to have", so putting it to Future. (But we will welcome a contribution!)

[CarnaViire]

I wonder how the team could improve the message in this case however, since they don't have a clean way to check for the issue before it throws AFAIK.

Well, I assume the check could be just a super-simplified version of the check the ActivatorUtilities themselves are using

https://github.com/dotnet/runtime/blob/ca0011a9698ec113535b710285bdda942ec5bd2b/src/libraries/Microsoft.Extensions.DependencyInjection.Abstractions/src/ActivatorUtilities.cs#L601

Yes, we cannot 100% guarantee that the activator would be able to create an instance later (due to e.g. problems with some other dependencies -- but it would give a proper error message), but we should be able to check that there's a constructor with HttpClient parameter on it 😅

(And yes @julealgon, typed clients are kind of a mess. As well as the factory itself. But the thing is we already have this mess, so we must live with it now. And it's widely used, so we must support it as well. Regardless whether or not new shiny DI features will get implemented.)

[CarnaViire]

UPD:

M.E.DI does NOT check the constructors at the registration time (e.g. I still can register a type with no public constructors at all, but it will throw on creation) -- so we should match.

This means it's even easier than I thought, because the exception in question can happen in HttpClientFactory only during the creation of the ObjectFactory for the Typed client, and only if there was no constructor with HttpClient -- so we just need to catch the exception and wrap into a meaningful one.

To leave some breadcrumbs to anyone interested, the ObjectFactory is created here: https://github.com/dotnet/runtime/blob/c9f7f954d4db70a1c819f6cf2ad1e0dffbd61ad5/src/libraries/Microsoft.Extensions.Http/src/DefaultTypedHttpClientFactory.cs#L38

[john-holden-1]

I don't know if this matters, but I ran into the same issue today and I found that the order in which you register your service and add a strongly typed HttpClient apparently matters.

using Microsoft.Extensions.DependencyInjection;

var services = new ServiceCollection();

// ERROR
//services.AddSingleton<TestClass>();
//services.AddHttpClient<TestClass>();

// NO ERROR
services.AddHttpClient<TestClass>();
services.AddSingleton<TestClass>();

var provider = services.BuildServiceProvider();
var testClass = provider.GetRequiredService<TestClass>();

public class TestClass {
    public TestClass(IHttpClientFactory httpClientFactory) {}
}

[julealgon]

@john-holden-1 when you call AddHttpClient<TestClass>, it registers your TestClass behind the scenes for you. When you "change the order" there, and add an additional AddSingleton<TestClass> after that, you are essentially overriding the registration for a single request. This is why you see no errors.

I'm pretty sure that if you replace your GetRequiredService<TestClass>() call with GetRequiredService<IEnumerable<TestClass>>() that you will once again see the issue, because it will then try to resolve both registrations, and fail on the one that was created by AddHttpClient with the exact same error.

[andrewjsaid]

the exception in question can happen in HttpClientFactory only during the creation of the ObjectFactory for the Typed client, and only if there was no constructor with HttpClient -- so we just need to catch the exception and wrap into a meaningful one.

@CarnaViire would the solution you reference have to match the specific message SR.Format(SR.CtorNotLocated, instanceType)? I ask because in theory anInvalidOperationException could occur if one of the constructor parameters is legit unregistered. In the example below where OtherService is not registered, we observe the following error, which is also InvalidOperationException and which is indeed helpful and unrelated to HttpClient registration internals.

Another option would be to subclass InvalidOperationException specifically for ActivatorUtilities not finding a constructor, but I assume that would need API review?

InvalidOperationException: Unable to resolve service for type 'OtherService' while attempting to activate 'MyService'.

using Microsoft.AspNetCore.Mvc;

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddHttpClient<MyService>();

var app = builder.Build();

app.MapGet("/hello", ([FromServices] MyService service) => service.Hello());

app.Run();

public class MyService(HttpClient httpClient, OtherService otherService)
{
    public string Hello()
    {
        return "World";
    }
}

public class OtherService { }

[CarnaViire]

would the solution you reference have to match the specific message

No, we won't need to @andrewjsaid.

These exceptions are happening on the different code paths (creating the ObjectFactory vs actually calling this ObjectFactory).

ActivatorUtilities.CreateFactory(...) would only throw if there was no public constructor containing all of the parameters provided, and the only one we're passing there is HttpClient (see DefaultTypedHttpClientFactory.cs):

// (simplified for display purposes)

Func<ObjectFactory> _createActivator = () =>
    ActivatorUtilities.CreateFactory(
        typeof(TClient),
        new Type[] { typeof(HttpClient), });

ObjectFactory Activator => LazyInitializer.EnsureInitialized(
                ....
                _createActivator)!;

You can see that in the callstacks:

1) No HttpClient in constructor -> throws while creating the ObjectFactory (ActivatorUtilities.CreateFactory)

System.InvalidOperationException: A suitable constructor for type 'MyService' could not be located. ....
   at ....
   at .....ActivatorUtilities.CreateFactory(Type instanceType, Type[] argumentTypes)
   at .....LazyInitializer.EnsureInitializedCore[T](T& target, Boolean& initialized, Object& syncLock, Func`1 valueFactory)
   at .....DefaultTypedHttpClientFactory`1.Cache.get_Activator()
   at .....DefaultTypedHttpClientFactory`1.CreateClient(HttpClient httpClient)
....

2) Other service cannot be resolved in constructor -> throws when calling the created ObjectFactory (lambda_method2)

System.InvalidOperationException: Unable to resolve service for type 'OtherService' while attempting to activate 'MyService'.
   at .....ActivatorUtilities.ThrowHelperUnableToResolveService(Type type, Type requiredBy)
   at lambda_method2(Closure, IServiceProvider, Object[])
   at .....DefaultTypedHttpClientFactory`1.CreateClient(HttpClient httpClient)
....