Skip to content
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

Ensuring required services are registered when using the server reflection #1732

Conversation

danielvieiravega
Copy link
Contributor

This PR solves the problem related in this issue.

It is being checked whether the services needed to use gRPC reflection have been added. If not, an InvalidOperationException is thrown, asking to call the IServiceCollection.AddGrpcReflection() method.

@JamesNK
Copy link
Member

JamesNK commented May 10, 2022

This is great. Exactly how I was thinking of improving the error message.

I'd like to have a unit test of this failing. There is an example of doing this here:

[Test]
public void MapGrpcService_WithoutServices_RaiseError()
{
// Arrange
var services = new ServiceCollection();
var routeBuilder = CreateTestEndpointRouteBuilder(services.BuildServiceProvider(validateScopes: true));
// Act & Assert
var ex = Assert.Throws<InvalidOperationException>(() => routeBuilder.MapGrpcService<Greeter.GreeterBase>())!;
Assert.AreEqual("Unable to find the required services. Please add all the required services by calling " +
"'IServiceCollection.AddGrpc' inside the call to 'ConfigureServices(...)' in the application startup code.", ex.Message);
}

@danielvieiravega
Copy link
Contributor Author

I added the test @JamesNK.

@JamesNK JamesNK merged commit 248c580 into grpc:master May 14, 2022
@JamesNK
Copy link
Member

JamesNK commented May 14, 2022

Perfect. Thanks!

@danielvieiravega danielvieiravega deleted the ensure-required-services-are-registered-server-reflection branch May 16, 2022 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants