Http2Connection: Wrap InvalidOperationExceptions (dotnet/corefx#39400)

* Http2Connection: Wrap InvalidOperationExceptions

To achieve parity with the v1.1 implementation. Fixes dotnet/corefx#39295

* do not shutdown connection on server side

* prevent disposal of client before server has established connection

Commit migrated from https://github.com/dotnet/corefx/commit/68e7999c846131bd8cd43899671fa6a4a7b5da53

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs
index b9c6a5d..c436745 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs
@@ -1600,7 +1600,8 @@ namespace System.Net.Http
 
                 if (e is IOException ||
                     e is ObjectDisposedException ||
-                    e is Http2ProtocolException)
+                    e is Http2ProtocolException ||
+                    e is InvalidOperationException)
                 {
                     replacementException = new HttpRequestException(SR.net_http_client_execution_error, _abortException ?? e);
                 }

diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs
index 59bda29..a055d0c 100644 (file)
--- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs
+++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs
@@ -1784,6 +1784,63 @@ namespace System.Net.Http.Functional.Tests
             });
         }
 
+        [ConditionalFact(nameof(SupportsAlpn))]
+        public async Task Http2Connection_Should_Wrap_HttpContent_InvalidOperationException()
+        {
+            // test for #39295
+            var throwingContent = new ThrowingContent(() => new InvalidOperationException());
+
+            var tcs = new TaskCompletionSource<bool>();
+            await Http2LoopbackServer.CreateClientAndServerAsync(async url =>
+            {
+                using (HttpClient client = CreateHttpClient())
+                {
+                    var request = new HttpRequestMessage(HttpMethod.Post, url);
+                    request.Version = new Version(2, 0);
+                    request.Content = throwingContent;
+
+                    HttpRequestException exn = await Assert.ThrowsAnyAsync<HttpRequestException>(async () => await client.SendAsync(request));
+                    Assert.IsType<InvalidOperationException>(exn.InnerException);
+                    await tcs.Task; // prevent disposal of client until server has completed operations
+                }
+            },
+            async server =>
+            {
+                await server.EstablishConnectionAsync();
+                tcs.SetResult(false);
+            });
+        }
+
+        [ConditionalFact(nameof(SupportsAlpn))]
+        public async Task Http2Connection_Should_Not_Wrap_HttpContent_CustomException()
+        {
+            // Assert existing HttpConnection behaviour in which custom HttpContent exception types are surfaced as-is
+            // c.f. https://github.com/dotnet/corefx/issues/39295#issuecomment-510569836
+
+            var throwingContent = new ThrowingContent(() => new CustomException());
+
+            var tcs = new TaskCompletionSource<bool>();
+            await Http2LoopbackServer.CreateClientAndServerAsync(async url =>
+            {
+                using (HttpClient client = CreateHttpClient())
+                {
+                    var request = new HttpRequestMessage(HttpMethod.Post, url);
+                    request.Version = new Version(2, 0);
+                    request.Content = throwingContent;
+
+                    await Assert.ThrowsAnyAsync<CustomException>(async () => await client.SendAsync(request));
+                    await tcs.Task; // prevent disposal of client until server has completed operations
+                }
+            },
+            async server =>
+            {
+                await server.EstablishConnectionAsync();
+                tcs.SetResult(false);
+            });
+        }
+
+        private class CustomException : Exception { }
+
         [Theory]
         [InlineData(true)]
         [InlineData(false)]

diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj b/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj
index ee768b4..2b38963 100644 (file)
--- a/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj
+++ b/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj
@@ -139,6 +139,7 @@
     <Compile Include="SyncBlockingContent.cs" />
     <Compile Include="TestHelper.cs" />
     <Compile Include="DefaultCredentialsTest.cs" />
+    <Compile Include="ThrowingContent.cs" />
   </ItemGroup>
   <ItemGroup Condition="'$(TargetGroup)' == 'netcoreapp'">
     <Compile Include="CustomContent.netcore.cs" />

diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.cs
new file mode 100644 (file)
index 0000000..aff18aa
--- /dev/null
+++ b/src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.cs
@@ -0,0 +1,32 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace System.Net.Http.Functional.Tests
+{
+    /// <summary>HttpContent that mocks exceptions on serialization.</summary>
+    public class ThrowingContent : HttpContent
+    {
+        private readonly Func<Exception> _exnFactory;
+        private readonly int _length;
+
+        public ThrowingContent(Func<Exception> exnFactory, int length = 10)
+        {
+            _exnFactory = exnFactory;
+            _length = length;
+        }
+
+        protected override Task SerializeToStreamAsync(Stream stream, TransportContext context)
+        {
+            throw _exnFactory();
+        }
+
+        protected override bool TryComputeLength(out long length)
+        {
+            length = _length;
+            return true;
+        }
+    }
+}