Skip to content

Improve error message when GetRequiredKeyedService() fails #114796

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 18, 2025
Merged

Improve error message when GetRequiredKeyedService() fails #114796

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions ...DependencyInjection.Specification.Tests/src/KeyedDependencyInjectionSpecificationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,28 @@ public void ResolveKeyedTransientFromScopeServiceProvider()
Assert.NotSame(serviceA1, serviceB1);
}

[Fact]
public void ResolveRequiredKeyedServiceThrowsIfNotFound()
{
var serviceCollection = new ServiceCollection();
var provider = CreateServiceProvider(serviceCollection);
var serviceKey = new object();

InvalidOperationException e;

e = Assert.Throws<InvalidOperationException>(() => provider.GetRequiredKeyedService<IService>(serviceKey));
VerifyException();

e = Assert.Throws<InvalidOperationException>(() => provider.GetRequiredKeyedService(typeof(IService), serviceKey));
VerifyException();

void VerifyException()
{
Assert.Contains(nameof(IService), e.Message);
Assert.Contains(serviceKey.GetType().FullName, e.Message);
}
}

[Fact]
public void ResolveKeyedServiceThrowsIfNotSupported()
{
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/Microsoft.Extensions.DependencyInjection/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,4 +195,7 @@
<data name="KeyedServiceAnyKeyUsedToResolveService" xml:space="preserve">
<value>KeyedService.AnyKey cannot be used to resolve a single service.</value>
</data>
<data name="NoKeyedServiceRegistered" xml:space="preserve">
<value>No keyed service for type '{0}' using key type '{1}' has been registered.</value>
Copy link
Member

@stephentoub stephentoub Apr 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to include a ToString()'d key, only its type? Seems like one of the most common types of keys is string, and that string value would be more useful than "System.String". Is it that we're concerned about leaking details in the error message?

Copy link
Contributor Author

@steveharter steveharter Apr 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We typically don't show disclose (possible) user data, but type names are OK.

</data>
</root>
14 changes: 14 additions & 0 deletions src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ThrowHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,19 @@ internal static void ThrowInvalidOperationException_KeyedServiceAnyKeyUsedToReso
{
throw new InvalidOperationException(SR.Format(SR.KeyedServiceAnyKeyUsedToResolveService, nameof(IServiceProvider), nameof(IServiceScopeFactory)));
}

[DoesNotReturn]
[MethodImpl(MethodImplOptions.NoInlining)]
internal static void ThrowInvalidOperationException_NoServiceRegistered(Type serviceType)
{
throw new InvalidOperationException(SR.Format(SR.NoServiceRegistered, serviceType));
}

[DoesNotReturn]
[MethodImpl(MethodImplOptions.NoInlining)]
internal static void ThrowInvalidOperationException_NoKeyedServiceRegistered(Type serviceType, Type keyType)
{
throw new InvalidOperationException(SR.Format(SR.NoKeyedServiceRegistered, serviceType, keyType));
}
}
}
10 changes: 9 additions & 1 deletion src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,16 @@ internal object GetRequiredKeyedService(Type serviceType, object? serviceKey, Se
object? service = GetKeyedService(serviceType, serviceKey, serviceProviderEngineScope);
if (service == null)
{
throw new InvalidOperationException(SR.Format(SR.NoServiceRegistered, serviceType));
if (serviceKey is null)
{
ThrowHelper.ThrowInvalidOperationException_NoServiceRegistered(serviceType);
}
else
{
ThrowHelper.ThrowInvalidOperationException_NoKeyedServiceRegistered(serviceType, serviceKey.GetType());
}
}

return service;
}

Expand Down
Loading