Clean up HTTP/2 protocol exception handling

src/System.Net.Http/src/Resources/Strings.resx

@@ -443,6 +443,9 @@
   <data name="net_http_winhttp_error" xml:space="preserve">
     <value>Error {0} calling {1}, '{2}'.</value>
   </data>
+  <data name="net_http_http2_protocol_error" xml:space="preserve">
+    <value>The HTTP/2 request failed with protocol error '{0}' (0x{1}).</value>
+  </data>
   <data name="net_MethodNotImplementedException" xml:space="preserve">
     <value>This method is not implemented by this class.</value>
   </data>

src/System.Net.Http/src/System.Net.Http.csproj

@@ -140,6 +140,8 @@
     <Compile Include="System\Net\Http\SocketsHttpHandler\EmptyReadStream.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\ExposedSocketNetworkStream.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\Http2Connection.cs" />
+    <Compile Include="System\Net\Http\SocketsHttpHandler\Http2ProtocolErrorCode.cs" />
+    <Compile Include="System\Net\Http\SocketsHttpHandler\Http2ProtocolException.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\Http2Stream.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpAuthenticatedConnectionHandler.cs" />
     <Compile Include="System\Net\Http\SocketsHttpHandler\HttpBaseStream.cs" />

src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs

@@ -162,9 +162,9 @@ private async Task FlushOutgoingBytesAsync()
             {
                 await _stream.WriteAsync(_outgoingBuffer.ActiveMemory).ConfigureAwait(false);
             }
-            catch (Exception)
+            catch (Exception e)
             {
-                Abort();
+                Abort(e);
                 throw;
             }
             finally
@@ -251,9 +251,9 @@ private async void ProcessIncomingFrames()
                     }
                 }
             }
-            catch (Exception)
+            catch (Exception e)
             {
-                Abort();
+                Abort(e);
             }
         }
 
@@ -604,12 +604,11 @@ private void ProcessRstStreamFrame(FrameHeader frameHeader)
                 return;
             }
 
-            _incomingBuffer.Discard(frameHeader.Length);
+            var protocolError = (Http2ProtocolErrorCode)BinaryPrimitives.ReadInt32BigEndian(_incomingBuffer.ActiveSpan);
 
-            // CONSIDER: We ignore the error code in the RST_STREAM frame.
-            // We could read this and report it to the user as part of the request exception.
+            _incomingBuffer.Discard(frameHeader.Length);
 
-            http2Stream.OnResponseAbort();
+            http2Stream.OnResponseAbort(new Http2ProtocolException(protocolError));
 
             RemoveStream(http2Stream);
         }
@@ -630,9 +629,10 @@ private void ProcessGoAwayFrame(FrameHeader frameHeader)
                 throw new Http2ProtocolException(Http2ProtocolErrorCode.ProtocolError);
             }
 
-            int lastValidStream = (int)((uint)((_incomingBuffer.ActiveSpan[0] << 24) | (_incomingBuffer.ActiveSpan[1] << 16) | (_incomingBuffer.ActiveSpan[2] << 8) | _incomingBuffer.ActiveSpan[3]) & 0x7FFFFFFF);
+            int lastValidStream = (int)(BinaryPrimitives.ReadUInt32BigEndian(_incomingBuffer.ActiveSpan) & 0x7FFFFFFF);
+            var errorCode = (Http2ProtocolErrorCode)BinaryPrimitives.ReadInt32BigEndian(_incomingBuffer.ActiveSpan.Slice(sizeof(int)));
 
-            AbortStreams(lastValidStream);
+            AbortStreams(lastValidStream, new Http2ProtocolException(errorCode));
 
             _incomingBuffer.Discard(frameHeader.Length);
         }
@@ -1113,11 +1113,11 @@ private void WriteFrameHeader(FrameHeader frameHeader)
             _outgoingBuffer.Commit(FrameHeader.Size);
         }
 
-        private void Abort()
+        private void Abort(Exception abortException)
         {
             // The connection has failed, e.g. failed IO or a connection-level frame error.
             // Abort all streams and cause further processing to fail.
-            AbortStreams(0);
+            AbortStreams(0, abortException);
         }
 
         private bool IsAborted()
@@ -1160,7 +1160,7 @@ public bool IsExpired(int nowTicks,
             return LifetimeExpired(nowTicks, connectionLifetime);
         }
 
-        private void AbortStreams(int lastValidStream)
+        private void AbortStreams(int lastValidStream, Exception abortException)
         {
             lock (SyncObject)
             {
@@ -1178,7 +1178,7 @@ private void AbortStreams(int lastValidStream)
 
                     if (streamId > lastValidStream)
                     {
-                        kvp.Value.OnResponseAbort();
+                        kvp.Value.OnResponseAbort(abortException);
 
                         _httpStreams.Remove(kvp.Value.StreamId);
                     }
@@ -1338,8 +1338,6 @@ private enum SettingId : ushort
 
         public sealed override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
         {
-            // TODO: ISSUE 31310: Cancellation support
-
             Http2Stream http2Stream = null;
             try
             {
@@ -1354,20 +1352,13 @@ public sealed override async Task<HttpResponseMessage> SendAsync(HttpRequestMess
             }
             catch (Exception e)
             {
-                http2Stream?.Dispose();
+                Exception replacementException = null;
 
-                if (e is IOException)
-                {
-                    throw new HttpRequestException(SR.net_http_client_execution_error, e);
-                }
-                else if (e is ObjectDisposedException)
-                {
-                    throw new HttpRequestException(SR.net_http_client_execution_error, e);
-                }
-                else if (e is Http2ProtocolException)
+                if (e is IOException ||
+                    e is ObjectDisposedException ||
+                    e is Http2ProtocolException)
                 {
-                    // ISSUE 31315: Determine if/how to expose HTTP2 error codes
-                    throw new HttpRequestException(SR.net_http_client_execution_error, e);
+                    replacementException = new HttpRequestException(SR.net_http_client_execution_error, e);
                 }
                 else if (e is OperationCanceledException oce)
                 {
@@ -1377,17 +1368,19 @@ public sealed override async Task<HttpResponseMessage> SendAsync(HttpRequestMess
                         http2Stream.Cancel();
                     }
 
-                    if (oce.CancellationToken == cancellationToken)
+                    if (oce.CancellationToken != cancellationToken)
                     {
-                        throw;
+                        replacementException = new OperationCanceledException(oce.Message, oce, cancellationToken);
                     }
-
-                    throw new OperationCanceledException(cancellationToken);
                 }
-                else
+
+                http2Stream?.Dispose();
+
+                if (replacementException != null)
                 {
-                    throw;
+                    throw replacementException;
                 }
+                throw;
             }
 
             return http2Stream.Response;
@@ -1401,7 +1394,7 @@ private Http2Stream AddStream(HttpRequestMessage request)
                 {
                     // Throw a retryable request exception. This will cause retry logic to kick in
                     // and perform another connection attempt. The user should never see this exception.
-                    throw new HttpRequestException(null, null, true);
+                    throw new HttpRequestException(null, null, allowRetry: true);
                 }
 
                 int streamId = _nextStream;
@@ -1443,40 +1436,7 @@ private void RemoveStream(Http2Stream http2Stream)
             }
         }
 
-        // TODO: ISSUE 31315: Should this be public?
-        internal enum Http2ProtocolErrorCode
-        {
-            NoError = 0x0,
-            ProtocolError = 0x1,
-            InternalError = 0x2,
-            FlowControlError = 0x3,
-            SettingsTimeout = 0x4,
-            StreamClosed = 0x5,
-            FrameSizeError = 0x6,
-            RefusedStream = 0x7,
-            Cancel = 0x8,
-            CompressionError = 0x9,
-            ConnectError = 0xa,
-            EnhanceYourCalm = 0xb,
-            InadequateSecurity = 0xc,
-            Http11Required = 0xd
-        }
-
-        // TODO: ISSUE 31315: Should this be public?
-        internal class Http2ProtocolException : Exception
-        {
-            private readonly Http2ProtocolErrorCode _errorCode;
-
-            public Http2ProtocolException(Http2ProtocolErrorCode errorCode)
-                : base($"Http2 Protocol Error, errorCode = {errorCode}")
-            {
-                _errorCode = errorCode;
-            }
-
-            public Http2ProtocolErrorCode ErrorCode => _errorCode;
-        }
-
-        internal static async ValueTask<int> ReadAtLeastAsync(Stream stream, Memory<byte> buffer, int minReadBytes)
+        private static async ValueTask<int> ReadAtLeastAsync(Stream stream, Memory<byte> buffer, int minReadBytes)
         {
             Debug.Assert(buffer.Length >= minReadBytes);
 

src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2ProtocolErrorCode.cs

@@ -0,0 +1,44 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+namespace System.Net.Http
+{
+    // NOTE: If any additional error codes are added here, they should also be added to Http2ProtocolException's mapping.
+
+    /// <summary>
+    /// Error codes defined by the HTTP/2 protocol, used in RST_STREAM and GOAWAY frames to convey the reasons for the stream or connection error.
+    /// https://http2.github.io/http2-spec/#PROTOCOL_ERROR
+    /// </summary>
+    internal enum Http2ProtocolErrorCode
+    {
+        /// <summary>The associated condition is not a result of an error.</summary>
+        NoError = 0x0,
+        /// <summary>The endpoint detected an unspecific protocol error. This error is for use when a more specific error code is not available.</summary>
+        ProtocolError = 0x1,
+        /// <summary>The endpoint encountered an unexpected internal error.</summary>
+        InternalError = 0x2,
+        /// <summary>The endpoint detected that its peer violated the flow-control protocol.</summary>
+        FlowControlError = 0x3,
+        /// <summary>The endpoint sent a SETTINGS frame but did not receive a response in a timely manner.</summary>
+        SettingsTimeout = 0x4,
+        /// <summary>The endpoint received a frame after a stream was half-closed.</summary>
+        StreamClosed = 0x5,
+        /// <summary>The endpoint received a frame with an invalid size.</summary>
+        FrameSizeError = 0x6,
+        /// <summary>The endpoint refused the stream prior to performing any application processing.</summary>
+        RefusedStream = 0x7,
+        /// <summary>Used by the endpoint to indicate that the stream is no longer needed.</summary>
+        Cancel = 0x8,
+        /// <summary>The endpoint is unable to maintain the header compression context for the connection.</summary>
+        CompressionError = 0x9,
+        /// <summary>The connection established in response to a CONNECT request was reset or abnormally closed.</summary>
+        ConnectError = 0xa,
+        /// <summary>The endpoint detected that its peer is exhibiting a behavior that might be generating excessive load.</summary>
+        EnhanceYourCalm = 0xb,
+        /// <summary>The underlying transport has properties that do not meet minimum security requirements.</summary>
+        InadequateSecurity = 0xc,
+        /// <summary>The endpoint requires that HTTP/1.1 be used instead of HTTP/2.</summary>
+        Http11Required = 0xd
+    }
+}

src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2ProtocolException.cs

@@ -0,0 +1,69 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Diagnostics;
+using System.Runtime.Serialization;
+
+namespace System.Net.Http
+{
+    [Serializable]
+    internal sealed class Http2ProtocolException : Exception
+    {
+        public Http2ProtocolException(Http2ProtocolErrorCode protocolError)
+            : base(SR.Format(SR.net_http_http2_protocol_error, GetName(protocolError), ((int)protocolError).ToString("x")))
+        {
+            ProtocolError = protocolError;
+        }
+
+        private Http2ProtocolException(SerializationInfo info, StreamingContext context) : base(info, context)
+        {
+            ProtocolError = (Http2ProtocolErrorCode)info.GetInt32(nameof(ProtocolError));
+        }
+
+        public override void GetObjectData(SerializationInfo info, StreamingContext context)
+        {
+            info.AddValue(nameof(ProtocolError), (int)ProtocolError);
+            base.GetObjectData(info, context);
+        }
+
+        internal Http2ProtocolErrorCode ProtocolError { get; }
+
+        private static string GetName(Http2ProtocolErrorCode code)
+        {
+            // These strings are the names used in the HTTP2 spec and should not be localized.
+            switch (code)
+            {
+                case Http2ProtocolErrorCode.NoError:
+                    return "NO_ERROR";
+                default: // any unrecognized error code is treated as a protocol error
+                case Http2ProtocolErrorCode.ProtocolError:
+                    return "PROTOCOL_ERROR";
+                case Http2ProtocolErrorCode.InternalError:
+                    return "INTERNAL_ERROR";
+                case Http2ProtocolErrorCode.FlowControlError:
+                    return "FLOW_CONTROL_ERROR";
+                case Http2ProtocolErrorCode.SettingsTimeout:
+                    return "SETTINGS_TIMEOUT";
+                case Http2ProtocolErrorCode.StreamClosed:
+                    return "STREAM_CLOSED";
+                case Http2ProtocolErrorCode.FrameSizeError:
+                    return "FRAME_SIZE_ERROR";
+                case Http2ProtocolErrorCode.RefusedStream:
+                    return "REFUSED_STREAM";
+                case Http2ProtocolErrorCode.Cancel:
+                    return "CANCEL";
+                case Http2ProtocolErrorCode.CompressionError:
+                    return "COMPRESSION_ERROR";
+                case Http2ProtocolErrorCode.ConnectError:
+                    return "CONNECT_ERROR";
+                case Http2ProtocolErrorCode.EnhanceYourCalm:
+                    return "ENHANCE_YOUR_CALM";
+                case Http2ProtocolErrorCode.InadequateSecurity:
+                    return "INADEQUATE_SECURITY";
+                case Http2ProtocolErrorCode.Http11Required:
+                    return "HTTP_1_1_REQUIRED";
+            }
+        }
+    }
+}

src/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs

@@ -43,6 +43,7 @@ private enum StreamState : byte
 
             private StreamState _state;
             private bool _disposed;
+            private Exception _abortException;
 
             /// <summary>The core logic for the IValueTaskSource implementation.</summary>
             private ManualResetValueTaskSourceCore<bool> _waitSource = new ManualResetValueTaskSourceCore<bool> { RunContinuationsAsynchronously = true }; // mutable struct, do not make this readonly
@@ -262,7 +263,7 @@ public void OnResponseData(ReadOnlySpan<byte> buffer, bool endStream)
                 }
             }
 
-            public void OnResponseAbort()
+            public void OnResponseAbort(Exception abortException)
             {
                 bool signalWaiter;
                 lock (SyncObject)
@@ -277,6 +278,7 @@ public void OnResponseAbort()
                         return;
                     }
 
+                    _abortException = abortException;
                     _state = StreamState.Aborted;
 
                     signalWaiter = _hasWaiter;
@@ -300,7 +302,7 @@ public void OnResponseAbort()
 
                     if (_state == StreamState.Aborted)
                     {
-                        throw new IOException(SR.net_http_request_aborted);
+                        throw new IOException(SR.net_http_request_aborted, _abortException);
                     }
                     else if (_state == StreamState.ExpectingHeaders)
                     {
@@ -393,7 +395,7 @@ private void ExtendWindow(int amount)
                     }
                     else if (_state == StreamState.Aborted)
                     {
-                        throw new IOException(SR.net_http_request_aborted);
+                        throw new IOException(SR.net_http_request_aborted, _abortException);
                     }
 
                     Debug.Assert(_state == StreamState.ExpectingData || _state == StreamState.ExpectingTrailingHeaders);
@@ -493,6 +495,7 @@ public void Cancel()
                 lock (SyncObject)
                 {
                     Task ignored = _connection.SendRstStreamAsync(_streamId, Http2ProtocolErrorCode.Cancel);
+                    _abortException = new OperationCanceledException();
                     _state = StreamState.Aborted;
 
                     signalWaiter = _hasWaiter;