From 3c8ac3fc8bca2ebcc4cd4b371bbebe8438528a7c Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Mon, 16 Jan 2023 20:50:12 +0800 Subject: [PATCH 1/4] Don't warn about Android native handler with grpc-web --- src/Grpc.Net.Client/GrpcChannel.cs | 6 ++- .../Grpc.Net.Client.Tests/GrpcChannelTests.cs | 39 ++++++++++++++++++- .../Infrastructure/GrpcWebHandler.cs | 26 +++++++++++++ 3 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 test/Grpc.Net.Client.Tests/Infrastructure/GrpcWebHandler.cs diff --git a/src/Grpc.Net.Client/GrpcChannel.cs b/src/Grpc.Net.Client/GrpcChannel.cs index 4153e74e7..a64641e63 100644 --- a/src/Grpc.Net.Client/GrpcChannel.cs +++ b/src/Grpc.Net.Client/GrpcChannel.cs @@ -431,13 +431,15 @@ private HttpMessageInvoker CreateInternalHttpInvoker(HttpMessageHandler? handler // in advanced gRPC scenarios. We want Android to use SocketsHttpHandler. Throw an error if: // 1. Client is running on Android. // 2. Channel is created with HttpClientHandler. - // 3. UseNativeHttpHandler switch is true. + // 3. Channel is not using GrpcWebHandler. grpc-web is compatible with the native handler. + // 4. UseNativeHttpHandler switch is true. if (OperatingSystem.IsAndroid) { // GetHttpHandlerType recurses through DelegatingHandlers that may wrap the HttpClientHandler. var httpClientHandler = HttpRequestHelpers.GetHttpHandlerType(handler); + var grpcWebHandler = HttpRequestHelpers.GetHttpHandlerType(handler, "Grpc.Net.Client.Web.GrpcWebHandler"); - if (httpClientHandler != null && RuntimeHelpers.QueryRuntimeSettingSwitch("System.Net.Http.UseNativeHttpHandler", defaultValue: false)) + if (httpClientHandler != null && grpcWebHandler == null && RuntimeHelpers.QueryRuntimeSettingSwitch("System.Net.Http.UseNativeHttpHandler", defaultValue: false)) { throw new InvalidOperationException("The channel configuration isn't valid on Android devices. " + "The channel is configured to use HttpClientHandler and Android's native HTTP/2 library. " + diff --git a/test/Grpc.Net.Client.Tests/GrpcChannelTests.cs b/test/Grpc.Net.Client.Tests/GrpcChannelTests.cs index 16128ef16..afe7cb031 100644 --- a/test/Grpc.Net.Client.Tests/GrpcChannelTests.cs +++ b/test/Grpc.Net.Client.Tests/GrpcChannelTests.cs @@ -26,7 +26,7 @@ using Microsoft.Extensions.Logging.Testing; using NUnit.Framework; using Grpc.Net.Client.Internal; -using System.Net; +using Grpc.Net.Client.Web; #if SUPPORT_LOAD_BALANCING using Grpc.Net.Client.Balancer; using Grpc.Net.Client.Balancer.Internal; @@ -402,7 +402,7 @@ public async Task Dispose_CalledWhileActiveCalls_ActiveCallsDisposed() Assert.AreEqual(0, channel.ActiveCalls.Count); } - [TestCase(null)] + [TestCase(true)] [TestCase(false)] public void HttpHandler_HttpClientHandlerOverNativeOnAndroid_ThrowError(bool useDelegatingHandlers) { @@ -443,6 +443,41 @@ public void HttpHandler_HttpClientHandlerOverNativeOnAndroid_ThrowError(bool use } } + [TestCase(true)] + [TestCase(false)] + public void HttpHandler_HttpClientHandlerOverNativeOnAndroid_HasGrpcWebHandler_ThrowError(bool useDelegatingHandlers) + { + // Arrange + AppContext.SetSwitch("System.Net.Http.UseNativeHttpHandler", true); + + try + { + var services = new ServiceCollection(); + services.AddSingleton(new TestOperatingSystem { IsAndroid = true }); + + HttpMessageHandler handler = new HttpClientHandler(); + if (useDelegatingHandlers) + { + handler = new TestDelegatingHandler(handler); + } + handler = new GrpcWebHandler(handler); + + var channel = GrpcChannel.ForAddress("https://localhost", new GrpcChannelOptions + { + HttpHandler = handler, + ServiceProvider = services.BuildServiceProvider() + }); + + // Assert + Assert.IsTrue(channel.OperatingSystem.IsAndroid); + } + finally + { + // Reset switch for other tests. + AppContext.SetSwitch("System.Net.Http.UseNativeHttpHandler", false); + } + } + private class TestDelegatingHandler : DelegatingHandler { public TestDelegatingHandler(HttpMessageHandler innerHandler) : base(innerHandler) diff --git a/test/Grpc.Net.Client.Tests/Infrastructure/GrpcWebHandler.cs b/test/Grpc.Net.Client.Tests/Infrastructure/GrpcWebHandler.cs new file mode 100644 index 000000000..df5cb3b46 --- /dev/null +++ b/test/Grpc.Net.Client.Tests/Infrastructure/GrpcWebHandler.cs @@ -0,0 +1,26 @@ +#region Copyright notice and license + +// Copyright 2019 The gRPC Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#endregion + +namespace Grpc.Net.Client.Web; + +public class GrpcWebHandler : DelegatingHandler +{ + public GrpcWebHandler(HttpMessageHandler innerHandler) : base(innerHandler) + { + } +} From 839c7e4ff72138c4af178df9cd56c634e5660d9b Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 18 Jan 2023 06:06:20 +0800 Subject: [PATCH 2/4] Update test/Grpc.Net.Client.Tests/Infrastructure/GrpcWebHandler.cs --- test/Grpc.Net.Client.Tests/Infrastructure/GrpcWebHandler.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Grpc.Net.Client.Tests/Infrastructure/GrpcWebHandler.cs b/test/Grpc.Net.Client.Tests/Infrastructure/GrpcWebHandler.cs index df5cb3b46..10c002615 100644 --- a/test/Grpc.Net.Client.Tests/Infrastructure/GrpcWebHandler.cs +++ b/test/Grpc.Net.Client.Tests/Infrastructure/GrpcWebHandler.cs @@ -18,6 +18,7 @@ namespace Grpc.Net.Client.Web; +// Use a GrpcWebHandler type here with the same name and namespace because Grpc.Net.Client.Tests doesn't reference Grpc.Net.Client.Web. public class GrpcWebHandler : DelegatingHandler { public GrpcWebHandler(HttpMessageHandler innerHandler) : base(innerHandler) From 7b75a4af000d9790fc589c3b05eef3d7504f09a2 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 18 Jan 2023 06:28:41 +0800 Subject: [PATCH 3/4] PR feedback --- .../Grpc.Net.Client.Tests.csproj | 1 + .../Infrastructure/GrpcWebHandler.cs | 27 ------------------- 2 files changed, 1 insertion(+), 27 deletions(-) delete mode 100644 test/Grpc.Net.Client.Tests/Infrastructure/GrpcWebHandler.cs diff --git a/test/Grpc.Net.Client.Tests/Grpc.Net.Client.Tests.csproj b/test/Grpc.Net.Client.Tests/Grpc.Net.Client.Tests.csproj index a2c890199..370eafae7 100644 --- a/test/Grpc.Net.Client.Tests/Grpc.Net.Client.Tests.csproj +++ b/test/Grpc.Net.Client.Tests/Grpc.Net.Client.Tests.csproj @@ -30,6 +30,7 @@ + diff --git a/test/Grpc.Net.Client.Tests/Infrastructure/GrpcWebHandler.cs b/test/Grpc.Net.Client.Tests/Infrastructure/GrpcWebHandler.cs deleted file mode 100644 index 10c002615..000000000 --- a/test/Grpc.Net.Client.Tests/Infrastructure/GrpcWebHandler.cs +++ /dev/null @@ -1,27 +0,0 @@ -#region Copyright notice and license - -// Copyright 2019 The gRPC Authors -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#endregion - -namespace Grpc.Net.Client.Web; - -// Use a GrpcWebHandler type here with the same name and namespace because Grpc.Net.Client.Tests doesn't reference Grpc.Net.Client.Web. -public class GrpcWebHandler : DelegatingHandler -{ - public GrpcWebHandler(HttpMessageHandler innerHandler) : base(innerHandler) - { - } -} From 901f693a67ee879b47aa90857038f5d1d4083070 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 18 Jan 2023 07:59:45 +0800 Subject: [PATCH 4/4] PR feedback --- test/Grpc.Net.Client.Tests/GrpcChannelTests.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/Grpc.Net.Client.Tests/GrpcChannelTests.cs b/test/Grpc.Net.Client.Tests/GrpcChannelTests.cs index afe7cb031..5dbc7a747 100644 --- a/test/Grpc.Net.Client.Tests/GrpcChannelTests.cs +++ b/test/Grpc.Net.Client.Tests/GrpcChannelTests.cs @@ -415,6 +415,8 @@ public void HttpHandler_HttpClientHandlerOverNativeOnAndroid_ThrowError(bool use services.AddSingleton(new TestOperatingSystem { IsAndroid = true }); HttpMessageHandler handler = new HttpClientHandler(); + + // Add an extra handler to verify that test successfully recurses down custom handlers. if (useDelegatingHandlers) { handler = new TestDelegatingHandler(handler); @@ -456,11 +458,13 @@ public void HttpHandler_HttpClientHandlerOverNativeOnAndroid_HasGrpcWebHandler_T services.AddSingleton(new TestOperatingSystem { IsAndroid = true }); HttpMessageHandler handler = new HttpClientHandler(); + handler = new GrpcWebHandler(handler); + + // Add an extra handler to verify that test successfully recurses down custom handlers. if (useDelegatingHandlers) { handler = new TestDelegatingHandler(handler); } - handler = new GrpcWebHandler(handler); var channel = GrpcChannel.ForAddress("https://localhost", new GrpcChannelOptions {