From 1b56ae42cb80fb449490f941324682505922be5d Mon Sep 17 00:00:00 2001 From: Geoff Kizer Date: Mon, 5 Aug 2019 09:02:47 -0700 Subject: [PATCH] fix exceptions thrown from SendHeadersAsync to allow retry when appropriate, and surface a useful error when retry is not allowed (dotnet/corefx#39985) Commit migrated from https://github.com/dotnet/corefx/commit/4e7d0c32d9cedc9d1436c3d71eec8ac96ed0064d --- .../System.Net.Http/src/Resources/Strings.resx | 3 ++ .../Net/Http/SocketsHttpHandler/Http2Connection.cs | 45 ++++++++++++++++++---- 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx index dafd9d1..c603c7a9 100644 --- a/src/libraries/System.Net.Http/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx @@ -525,4 +525,7 @@ Dynamic table size update to {0} bytes exceeds limit of {1} bytes. + + The server shut down the connection. + \ No newline at end of file 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 8779d55..50e90e8 100644 --- 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 @@ -1085,6 +1085,36 @@ namespace System.Net.Http } } + private HttpRequestException GetShutdownException() + { + Debug.Assert(Monitor.IsEntered(SyncObject)); + + // Throw a retryable exception that will allow this unprocessed request to be processed on a new connection. + // In rare cases, such as receiving GOAWAY immediately after connection establishment, we will not + // actually retry the request, so we must give a useful exception here for these cases. + + Exception innerException; + if (_abortException != null) + { + innerException = _abortException; + } + else if (_lastStreamId != -1) + { + // We must have received a GOAWAY. + innerException = new IOException(SR.net_http_server_shutdown); + } + else + { + // We must either be disposed or out of stream IDs. + // Note that in this case, the exception should never be visible to the user (it should be retried). + Debug.Assert(_disposed || _nextStream == MaxStreamId); + + innerException = new ObjectDisposedException(nameof(Http2Connection)); + } + + return new HttpRequestException(SR.net_http_client_execution_error, innerException, allowRetry: true); + } + private async ValueTask SendHeadersAsync(HttpRequestMessage request, CancellationToken cancellationToken, bool mustFlush) { // We serialize usage of the header encoder and the header buffer. @@ -1113,14 +1143,13 @@ namespace System.Net.Http // Throw a retryable request exception if this is not result of some other error. // This will cause retry logic to kick in and perform another connection attempt. // The user should never see this exception. See also below. - if (_abortException != null) - { - throw new HttpRequestException(SR.net_http_client_execution_error, _abortException); - } - Debug.Assert(_disposed || _lastStreamId != -1); Debug.Assert(_httpStreams.Count == 0); - throw CreateRetryException(); + + lock (SyncObject) + { + throw GetShutdownException(); + } } try @@ -1135,7 +1164,7 @@ namespace System.Net.Http // We ran out of stream IDs or we raced between acquiring the connection from the pool and shutting down. // 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 CreateRetryException(); + throw GetShutdownException(); } streamId = _nextStream; @@ -1706,7 +1735,7 @@ namespace System.Net.Http // The connection is shutting down. // 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 CreateRetryException(); + throw GetShutdownException(); } Http2Stream http2Stream = new Http2Stream(request, this, streamId, _initialWindowSize); -- 2.7.4