Don't throw exception in ServiceDescriptor

[benjaminpetit]

Right now, when a ServiceDescriptor is not keyed, accessing all the keyed properties (KeyedImplementationInstance, KeyedImplementationType, KeyedImplementationFactory) will throw an InvalidOperationException.

Similary, when the ServiceDescriptor is not keyed, accessing all the non-keyed properties(ImplementationInstance, ImplementationType, ImplementationFactory) will also throw an InvalidOperationException`.

Ideally the application should check the value of IsKeyedService before accessing these properties, but applications that are not yet aware of keyed services break because of these exceptions.

We should remove this behavior. Note that applications that do not "keyed services aware" can still break because all the expected properties will be null, but it should cause less issues.

@halter73 @steveharter @davidfowl

[dotnet-policy-service]

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

[halter73]

What's motivating this? A clear exception when accessing the property seems better than a NullReferenceException later in the code. It's a way to ensure people trying to use containers without keyed service support get clear error messages.

[benjaminpetit]

Some app/dependenciy might scan the list of ServiceDescriptor for different reasons.

Check this issue for example: microsoft/ApplicationInsights-dotnet#2852

[ReubenBond]

I also think that null is better behavior than an exception. We had 3 nullable one-of properties previously. The best behavior now would be to have 6 (3 keyed, 3 non-keyed) in my opinion. Explosive properties are cumbersome to deal with.

Eg, the most common pattern which is triggering this bad behavior (I already fixed a similar issue in Microsoft.Identity.Web and the same issue existed in Dapr) is to have a marker instance in DI to signal that services have already been added. This common pattern blows up with the new .NET 9 behavior.

[dotnet-policy-service]

Added needs-breaking-change-doc-created label because this issue has the breaking-change label.

1. Create and link to this issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.

Tagging @dotnet/compat for awareness of the breaking change.

[rjgotten]

@halter73
What's motivating this?

The .NET ecosystem equivalent of the "don't break the web" collloquial tenet.

@halter73
A clear exception when accessing the property seems better than a NullReferenceException later in the code. It's a way to ensure people trying to use containers without keyed service support get clear error messages.

A better solution would have been for the existing implementation of the service collection IEnumerable<> interface to explicitly leave out the keyed services, and to have a separate IServiceCollection.Keyed which returns the keyed services.

Keyed and unkeyed services should be treated as distinct worlds anyway.
You obviously shouldn't be able to satisfy a keyed dependency with a unkeyed (or other keyed) one; and probably also the vice-versa: you should not be able to satisfy an unkeyed dependency with an explicit keyed dependency. At least this is how traditional DI/IoC containers including Microsoft's own Unity have treated this case historically.

And if you're absolutely intent on throwing an error and halting when keyed services are used with a container implementation that doesn't support them - then the right moment in time to do it would be when the provider is built up and returned to the framework. At that point, you can check whether the returned container implements the requisite interface, and whether IServiceCollection.Keyed contains any items.
If the former doesn't and the latter does: that's when you throw an exception. (Which could helpfully enumerate all the keyed types as well.)

---

Furthermore:
The fact that you have to throw an exception to begin with is the consequence of an asinine stupid decision that engineers responsible for the keyed services feature have been warned about at length before, when they chose to use a technical design for the consumption of those services based on attribute markings.

The consequence of this dumb choice is that DI containers are absolutely required to be aware of said attribute, or support of keyed services will break.

What should have been done - and Microsoft was told about, is to use an interface type such as IKeyed<TKey,TValue> and rely on consumers using a service-locator like lookup in class constructors to pull out the one name from the collection that they're interested in. E.g.

public class Dependent
{
  private readonly Dependency _dependency;
  public Dependent(IKeyed<string, Dependency> dependencies)
  {
    ArgumentNullException.ThrowIfNull(dependencies);
    _dependency = dependencies.GetRequired("foo");
  }
}

The implementation of IKeyed<,> being put to use could then dynamically have been predicated on whether the container would've had native support for real keyed services - announced by its IServiceProvider implementing the ancillary new interface for it. It supports real keyed services? It just pipes through to the service provider ancillary interface to grab the one named item directly from the container. It doesn't? It will be front-loaded with a ductyped implementation that internally tracked the registrations at registration time.

(This isn't hard to get right in any sense of the word. I built this type of faux keyed services on top of the .NET 6 version of service providers in approx. 4 hours, basic unit tests proving its viability included. And the project I'm involved in has in fact been using it happily at scale, in production, for 2+ years and counting.)

And then finally, if you want to ensure consumers are aware of the performance benefits of using a container that supports keyed services natively, you can include a one-time log message at either the INFO or WARN level. But the application itself would just work.

And you know what? It would also have been a good extension point for #100105 because you'd just have an IKeyed<TKey,TValue>.GetAll() and .GetAllKeys().

In short - the entire design for keyed services essentially just screwed the pooch, front to back.
And the right thing to do is to bin it and redo it.

[steveharter]

Closing as a duplicate of #95789