Skip to content

Assign primary handler before other configuration #2445

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

Merged
merged 7 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion Directory.Packages.props
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
<OpenTelemetryIntergationPackageVersion>1.8.1</OpenTelemetryIntergationPackageVersion>
<OpenTelemetryGrpcPackageVersion>1.8.0-beta.1</OpenTelemetryGrpcPackageVersion>
<MicrosoftExtensionsPackageVersion>8.0.0</MicrosoftExtensionsPackageVersion>
<MicrosoftExtensions6PackageVersion>6.0.0</MicrosoftExtensions6PackageVersion>
<MicrosoftExtensionsLtsPackageVersion>6.0.0</MicrosoftExtensionsLtsPackageVersion>
</PropertyGroup>
<ItemGroup>
<!-- ASP.NET Core -->
Expand All @@ -24,6 +24,7 @@
<PackageVersion Include="Microsoft.AspNetCore.Grpc.Swagger" Version="0.8.0" />

<!-- Extensions -->
<PackageVersion Include="Microsoft.Extensions.Http" Version="$(MicrosoftExtensionsPackageVersion)" />
<PackageVersion Include="Microsoft.Extensions.Logging" Version="$(MicrosoftExtensionsPackageVersion)" />
<PackageVersion Include="Microsoft.Extensions.Logging.Console" Version="$(MicrosoftExtensionsPackageVersion)" />
<PackageVersion Include="Microsoft.Extensions.Hosting" Version="$(MicrosoftExtensionsPackageVersion)" />
Expand Down
2 changes: 1 addition & 1 deletion src/Grpc.Net.Client/Grpc.Net.Client.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions" VersionOverride="$(MicrosoftExtensions6PackageVersion)" />
<PackageReference Include="Microsoft.Extensions.Logging.Abstractions" VersionOverride="$(MicrosoftExtensionsLtsPackageVersion)" />
</ItemGroup>

<ItemGroup>
Expand Down
5 changes: 4 additions & 1 deletion src/Grpc.Net.Client/GrpcChannel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,10 @@ private HttpMessageInvoker CreateInternalHttpInvoker(HttpMessageHandler? handler
// Decision to dispose invoker is controlled by _shouldDisposeHttpClient.
if (handler == null)
{
handler = HttpHandlerFactory.CreatePrimaryHandler();
if (!HttpHandlerFactory.TryCreatePrimaryHandler(out handler))
{
throw HttpHandlerFactory.CreateUnsupportedHandlerException();
}
}
else
{
Expand Down
2 changes: 1 addition & 1 deletion src/Grpc.Net.ClientFactory/Grpc.Net.ClientFactory.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@
<ItemGroup>
<!-- PrivateAssets set to None to ensure the build targets/props are propagated to parent project -->
<ProjectReference Include="..\Grpc.Net.Client\Grpc.Net.Client.csproj" PrivateAssets="None" />
<PackageReference Include="Microsoft.Extensions.Http" VersionOverride="$(MicrosoftExtensions6PackageVersion)" />
<PackageReference Include="Microsoft.Extensions.Http" VersionOverride="$(MicrosoftExtensionsLtsPackageVersion)" />
</ItemGroup>
</Project>
79 changes: 41 additions & 38 deletions src/Grpc.Net.ClientFactory/GrpcClientServiceExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#endregion

using System;
using System.Diagnostics.CodeAnalysis;
using Grpc.Net.ClientFactory;
using Grpc.Net.ClientFactory.Internal;
Expand Down Expand Up @@ -289,9 +290,7 @@ private static IHttpClientBuilder AddGrpcClientCore<
// because we access it by reaching into the service collection.
services.TryAddSingleton(new GrpcClientMappingRegistry());

IHttpClientBuilder clientBuilder = services.AddGrpcHttpClient<TClient>(name);

return clientBuilder;
return services.AddGrpcHttpClient<TClient>(name);
}

/// <summary>
Expand All @@ -306,25 +305,22 @@ private static IHttpClientBuilder AddGrpcHttpClient<
{
ArgumentNullThrowHelper.ThrowIfNull(services);

services
.AddHttpClient(name)
.ConfigurePrimaryHttpMessageHandler(() =>
{
// Set PrimaryHandler to null so we can track whether the user
// set a value or not. If they didn't set their own handler then
// one will be created by PostConfigure.
return null!;
});
var builder = services.AddHttpClient(name);

services.PostConfigure<HttpClientFactoryOptions>(name, options =>
builder.Services.AddTransient<TClient>(s =>
{
options.HttpMessageHandlerBuilderActions.Add(builder =>
var clientFactory = s.GetRequiredService<GrpcClientFactory>();
return clientFactory.CreateClient<TClient>(name);
});

// Insert primary handler before other configuration so there is the opportunity to override it.
// This should run before ConfigureDefaultHttpClient so the handler can be overriden in defaults.
var configurePrimaryHandler = ServiceDescriptor.Singleton<IConfigureOptions<HttpClientFactoryOptions>>(new ConfigureNamedOptions<HttpClientFactoryOptions>(name, options =>
{
options.HttpMessageHandlerBuilderActions.Add(b =>
{
if (builder.PrimaryHandler == null)
if (HttpHandlerFactory.TryCreatePrimaryHandler(out var handler))
{
// This will throw in .NET Standard 2.0 with a prompt that a user must set a handler.
// Because it throws it should only be called in PostConfigure if no handler has been set.
var handler = HttpHandlerFactory.CreatePrimaryHandler();
#if NET5_0_OR_GREATER
if (handler is SocketsHttpHandler socketsHttpHandler)
{
Expand All @@ -336,37 +332,34 @@ private static IHttpClientBuilder AddGrpcHttpClient<
}
#endif

builder.PrimaryHandler = handler;
b.PrimaryHandler = handler;
}
else
{
b.PrimaryHandler = UnsupportedHttpHandler.Instance;
}
});
});

var builder = new DefaultHttpClientBuilder(services, name);
}));
services.Insert(0, configurePrimaryHandler);

builder.Services.AddTransient<TClient>(s =>
// Some platforms don't have a built-in handler that supports gRPC.
// Validate that a handler was set by the app to after all configuration has run.
services.PostConfigure<HttpClientFactoryOptions>(name, options =>
{
var clientFactory = s.GetRequiredService<GrpcClientFactory>();
return clientFactory.CreateClient<TClient>(builder.Name);
options.HttpMessageHandlerBuilderActions.Add(builder =>
{
if (builder.PrimaryHandler == UnsupportedHttpHandler.Instance)
{
throw HttpHandlerFactory.CreateUnsupportedHandlerException();
}
});
});

ReserveClient(builder, typeof(TClient), name);

return builder;
}

private class DefaultHttpClientBuilder : IHttpClientBuilder
{
public DefaultHttpClientBuilder(IServiceCollection services, string name)
{
Services = services;
Name = name;
}

public string Name { get; }

public IServiceCollection Services { get; }
}

private static void ReserveClient(IHttpClientBuilder builder, Type type, string name)
{
var registry = (GrpcClientMappingRegistry?)builder.Services.Single(sd => sd.ServiceType == typeof(GrpcClientMappingRegistry)).ImplementationInstance;
Expand All @@ -384,4 +377,14 @@ private static void ReserveClient(IHttpClientBuilder builder, Type type, string

registry.NamedClientRegistrations[name] = type;
}

private sealed class UnsupportedHttpHandler : HttpMessageHandler
{
public static readonly UnsupportedHttpHandler Instance = new UnsupportedHttpHandler();

protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
return Task.FromException<HttpResponseMessage>(HttpHandlerFactory.CreateUnsupportedHandlerException());
}
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#region Copyright notice and license
#region Copyright notice and license

// Copyright 2019 The gRPC Authors
//
Expand All @@ -16,7 +16,6 @@

#endregion


namespace Grpc.Net.ClientFactory.Internal;

internal class GrpcClientMappingRegistry
Expand Down
22 changes: 16 additions & 6 deletions src/Shared/HttpHandlerFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,43 +16,53 @@

#endregion

using System.Diagnostics.CodeAnalysis;
using Grpc.Net.Client;

namespace Grpc.Shared;

internal static class HttpHandlerFactory
{
public static HttpMessageHandler CreatePrimaryHandler()
public static bool TryCreatePrimaryHandler([NotNullWhen(true)] out HttpMessageHandler? primaryHandler)
{
#if NET5_0_OR_GREATER
// If we're in .NET 5 and SocketsHttpHandler is supported (it's not in Blazor WebAssembly)
// then create SocketsHttpHandler with EnableMultipleHttp2Connections set to true. That will
// allow a gRPC channel to create new connections if the maximum allow concurrency is exceeded.
if (SocketsHttpHandler.IsSupported)
{
return new SocketsHttpHandler
primaryHandler = new SocketsHttpHandler
{
EnableMultipleHttp2Connections = true
};
return true;
}
#endif

#if NET462
// Create WinHttpHandler with EnableMultipleHttp2Connections set to true. That will
// allow a gRPC channel to create new connections if the maximum allow concurrency is exceeded.
return new WinHttpHandler
primaryHandler = new WinHttpHandler
{
EnableMultipleHttp2Connections = true
};
return true;
#elif !NETSTANDARD2_0
return new HttpClientHandler();
primaryHandler = new HttpClientHandler();
return true;
#else
primaryHandler = null;
return false;
#endif
}

public static Exception CreateUnsupportedHandlerException()
{
var message =
$"gRPC requires extra configuration on .NET implementations that don't support gRPC over HTTP/2. " +
$"An HTTP provider must be specified using {nameof(GrpcChannelOptions)}.{nameof(GrpcChannelOptions.HttpHandler)}." +
$"The configured HTTP provider must either support HTTP/2 or be configured to use gRPC-Web. " +
$"See https://aka.ms/aspnet/grpc/netstandard for details.";
throw new PlatformNotSupportedException(message);
#endif
return new PlatformNotSupportedException(message);
}
}
13 changes: 12 additions & 1 deletion test/FunctionalTests/Linker/LinkerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
// Skip running load running tests in debug configuration
#if !DEBUG

using System.Globalization;
using System.Reflection;
using System.Runtime.InteropServices;
using Grpc.AspNetCore.FunctionalTests.Linker.Helpers;
Expand Down Expand Up @@ -86,7 +87,17 @@ private async Task RunWebsiteAndCallWithClient(bool publishAot)
websiteProcess.Start(BuildStartPath(linkerTestsWebsitePath, "LinkerTestsWebsite"), arguments: null);
await websiteProcess.WaitForReadyAsync().TimeoutAfter(Timeout);

clientProcess.Start(BuildStartPath(linkerTestsClientPath, "LinkerTestsClient"), arguments: websiteProcess.ServerPort!.ToString());
string? clientArguments = null;
if (websiteProcess.ServerPort is {} serverPort)
{
clientArguments = serverPort.ToString(CultureInfo.InvariantCulture);
}
else
{
throw new InvalidOperationException("Website server port not available.");
}

clientProcess.Start(BuildStartPath(linkerTestsClientPath, "LinkerTestsClient"), arguments: clientArguments);
await clientProcess.WaitForExitAsync().TimeoutAfter(Timeout);
}
finally
Expand Down
4 changes: 1 addition & 3 deletions test/Grpc.Net.Client.Tests/Grpc.Net.Client.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,11 @@
<PackageReference Include="Google.Protobuf" />
<PackageReference Include="Grpc.Tools" PrivateAssets="All" />
<PackageReference Include="Microsoft.Extensions.Logging.Testing" />
<PackageReference Include="Microsoft.Extensions.Logging" VersionOverride="5.0.0" />
<PackageReference Include="Microsoft.Extensions.Logging" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)'=='net462'">
<Reference Include="System.Net.Http" />

<PackageReference Include="Microsoft.Bcl.AsyncInterfaces" />
</ItemGroup>

</Project>
76 changes: 76 additions & 0 deletions test/Grpc.Net.ClientFactory.Tests/DefaultGrpcClientFactoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,34 @@ public void CreateClient_Default_DefaultInvokerSet()
Assert.IsInstanceOf(typeof(HttpMessageInvoker), client.CallInvoker.Channel.HttpInvoker);
}

#if NET6_0_OR_GREATER
[Test]
public void CreateClient_Default_PrimaryHandlerIsSocketsHttpHandler()
{
// Arrange
HttpMessageHandler? clientPrimaryHandler = null;
var services = new ServiceCollection();
services
.AddGrpcClient<TestGreeterClient>(o => o.Address = new Uri("http://localhost"))
.ConfigurePrimaryHttpMessageHandler((primaryHandler, _) =>
{
clientPrimaryHandler = primaryHandler;
});

var serviceProvider = services.BuildServiceProvider(validateScopes: true);

var clientFactory = CreateGrpcClientFactory(serviceProvider);

// Act
var client = clientFactory.CreateClient<TestGreeterClient>(nameof(TestGreeterClient));

// Assert
Assert.NotNull(clientPrimaryHandler);
Assert.IsInstanceOf<SocketsHttpHandler>(clientPrimaryHandler);
Assert.IsTrue(((SocketsHttpHandler)clientPrimaryHandler!).EnableMultipleHttp2Connections);
}
#endif

[Test]
public void CreateClient_MatchingConfigurationBasedOnTypeName_ReturnConfiguration()
{
Expand Down Expand Up @@ -254,6 +282,54 @@ public void CreateClient_NoPrimaryHandlerNetStandard_ThrowError()
// Assert
Assert.AreEqual(@"gRPC requires extra configuration on .NET implementations that don't support gRPC over HTTP/2. An HTTP provider must be specified using GrpcChannelOptions.HttpHandler.The configured HTTP provider must either support HTTP/2 or be configured to use gRPC-Web. See https://aka.ms/aspnet/grpc/netstandard for details.", ex.Message);
}

[Test]
public void CreateClient_ConfigureDefaultAfter_Success()
{
// Arrange
var services = new ServiceCollection();
services
.AddGrpcClient<TestGreeterClient>(o => o.Address = new Uri("https://localhost"));

services.ConfigureHttpClientDefaults(builder =>
{
builder.ConfigurePrimaryHttpMessageHandler(() => new NullHttpHandler());
});

var serviceProvider = services.BuildServiceProvider(validateScopes: true);

var clientFactory = CreateGrpcClientFactory(serviceProvider);

// Act
var client = clientFactory.CreateClient<TestGreeterClient>(nameof(TestGreeterClient));

// Assert
Assert.IsNotNull(client);
}

[Test]
public void CreateClient_ConfigureDefaultBefore_Success()
{
// Arrange
var services = new ServiceCollection();

services.ConfigureHttpClientDefaults(builder =>
{
builder.ConfigurePrimaryHttpMessageHandler(() => new NullHttpHandler());
});

services.AddGrpcClient<TestGreeterClient>(o => o.Address = new Uri("https://localhost"));

var serviceProvider = services.BuildServiceProvider(validateScopes: true);

var clientFactory = CreateGrpcClientFactory(serviceProvider);

// Act
var client = clientFactory.CreateClient<TestGreeterClient>(nameof(TestGreeterClient));

// Assert
Assert.IsNotNull(client);
}
#endif

#if NET5_0_OR_GREATER
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
<PackageReference Include="Google.Protobuf" />
<PackageReference Include="Grpc.Tools" PrivateAssets="All" />
<PackageReference Include="Microsoft.Extensions.Logging.Testing" />
<PackageReference Include="Microsoft.Extensions.Http" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)'=='net462'">
Expand Down
2 changes: 1 addition & 1 deletion testassets/InteropTestsWebsite/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM mcr.microsoft.com/dotnet/nightly/sdk:8.0 AS build-env
FROM mcr.microsoft.com/dotnet/sdk:8.0 AS build-env
WORKDIR /app

# Copy everything
Expand Down
1 change: 0 additions & 1 deletion testassets/LinkerTestsWebsite/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,4 @@

app.MapGrpcService<GreeterService>();

app.Lifetime.ApplicationStarted.Register(() => Console.WriteLine("Application started. Press Ctrl+C to shut down."));
app.Run();