-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Error message for missing service registration is misleading when using AddHttpClient<> #104386
Comments
If I take your example code as-is and run it, it works just fine: no errors.
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 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 |
Thank you for the explanation about what happens under the hood. I see that in fact if I inject 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 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 Unfortunately (IMHO), Microsoft decided to create a custom solution specifically for the 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. |
Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection |
Tagging subscribers to this area: @dotnet/ncl |
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!) |
Well, I assume the check could be just a super-simplified version of the check the ActivatorUtilities themselves are using Line 601 in ca0011a
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.) |
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: runtime/src/libraries/Microsoft.Extensions.Http/src/DefaultTypedHttpClientFactory.cs Line 38 in c9f7f95
|
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.
|
@john-holden-1 when you call I'm pretty sure that if you replace your |
@CarnaViire would the solution you reference have to match the specific message Another option would be to subclass
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 { } |
No, we won't need to @andrewjsaid. These exceptions are happening on the different code paths (creating the ObjectFactory vs actually calling this ObjectFactory).
// (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:
|
Description
This is only a misleading error message. If a service is not registered it usually gives the error message:
However if we register a typed HttpClient for this service then the error message is misleading:
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.
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
The text was updated successfully, but these errors were encountered: