Implement HttpRequestError #88974

antonfirsov · 2023-07-17T01:08:53Z

Implements the proposal from #76644 (comment). I left out TransportError, since it would be a new error case with little value, requiring new error logic.

Fixes #76644, fixes #82168.

dotnet-issue-labeler · 2023-07-17T01:08:58Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

ghost · 2023-07-17T01:09:01Z

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Implements the proposal from #76644 (comment). I left out TransportError, since it would be a new error case with little value, requiring new error logic.

Fixes #76644, fixes #82168.

Author:	antonfirsov
Assignees:	-
Labels:	`area-System.Net.Http`
Milestone:	8.0.0

antonfirsov · 2023-07-17T01:18:47Z

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs


                    case Http3ErrorCode.RequestRejected:
                        // The server is rejecting the request without processing it, retry it on a different connection.
-                        throw new HttpRequestException(SR.net_http_request_aborted, ex, RequestRetryType.RetryOnConnectionFailure);
+                        throw new HttpRequestException(SR.net_http_request_aborted, ex, RequestRetryType.RetryOnConnectionFailure, httpRequestError: HttpRequestError.Unknown);


When can these cases occur? How can we test them? Can we return something better for HttpRequestError.Unknown? Same for the other cases where I'm using HttpRequestError.Unknown.

The way I understand it, these do not surface to the user, they mean that we should retry on a different connection (either different version, or just a brand new H/3 connection). Looking at the handling code (after some retries, they might bubble up though).
The first one gets wrapped:

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs

Line 1165 in b898a54

catch (HttpRequestException e) when (e.AllowRetry == RequestRetryType.RetryOnLowerHttpVersion)

The second won't:

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs

Line 1142 in b898a54

catch (HttpRequestException e) when (e.AllowRetry == RequestRetryType.RetryOnConnectionFailure)

But, it's the same thing as this:

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

Lines 2205 to 2206 in b898a54

private static void ThrowRetry(string message, Exception? innerException = null) =>

throw new HttpRequestException(message, innerException, allowRetry: RequestRetryType.RetryOnConnectionFailure);

which you don't touch in this PR, so do we need to touch these at all????

Also, you should be easily able to test it with our loopback set up:

RequestRejected:

runtime/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs

Line 384 in d4db2d9

public async Task SendAsync_RequestRejected_ClientRetries()

VersionFallback: seems like we don't have a test for it, or at least I cannot find anything.

I handled these cases in 9afb790, by using HttpRequestError.ProtocolError and ProtocolException as an inner exception for the exception that can surface, but there are some other HTTP/3 cases which are still use Unknown:

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs

Lines 277 to 281 in 2eb3a4a

catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted && _connection.AbortException != null)

{

// we close the connection, propagate the AbortException

throw new HttpRequestException(SR.net_http_client_execution_error, _connection.AbortException, httpRequestError: HttpRequestError.Unknown);

}

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs

Lines 291 to 295 in 2eb3a4a

else

{

Debug.Assert(_requestBodyCancellationSource.IsCancellationRequested);

throw new HttpRequestException(SR.net_http_request_aborted, ex, RequestRetryType.RetryOnConnectionFailure, httpRequestError: HttpRequestError.Unknown);

}

runtime/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs

Lines 302 to 309 in 2eb3a4a

catch (Exception ex)

{

_stream.Abort(QuicAbortDirection.Write, (long)Http3ErrorCode.InternalError);

if (ex is HttpRequestException)

{

throw;

}

throw new HttpRequestException(SR.net_http_client_execution_error, ex, httpRequestError: HttpRequestError.Unknown);

I don't know what can trigger those, and whether there is a better HttpRequestError we could use. /cc @wfurt

I recommend to address this after platform complete.

src/libraries/System.Net.Http/ref/System.Net.Http.cs

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

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs

MihaZupan · 2023-07-17T06:53:05Z

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/SocksHelper.cs

@@ -359,6 +359,7 @@ private static async ValueTask ReadToFillAsync(Stream stream, Memory<byte> buffe

            if (bytesRead < buffer.Length)
            {
+                // TODO: How should we categorize this? Seems more like a connection establishment than than InvalidResponse


How would errors during the establishment of a CONNECT proxy tunnel be surfaced (e.g. proxy sent a garbled response -- not just a valid but non-200 one)?

ProxyTunnelError's description also seems to fit this case (we got a connection but failed to agree with the proxy).

Added a new message for the topmost HttpRequestException and decided to leave inner SocksExceptions as they are.

src/libraries/System.Net.Http/src/System/Net/Http/HttpIOException.cs

ManickaP · 2023-07-17T09:52:42Z

src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestException.cs

+        /// <value>
+        /// The <see cref="Http.HttpRequestError"/> or <see langword="null"/> if the underlying <see cref="HttpMessageHandler"/> did not provide it.
+        /// </value>
+        public HttpRequestError? HttpRequestError { get; }


Why is this nullable if we have Unknown, or the other way around?

null means that the underlying handler did not implement the error, while Unknown means generic/uncategorized error.

IMHO making difference between the two is very impractical and can be a source of problems, but that's where this ended after reacting to concerns in #76644 (comment) by removing nullability and defining a default 0 error code, and then agreeing with the advice from the API review group that nullable actually makes sense.

My recommendation would be to have a follow-up discussion on this and potentially remove Unknown in RC1.

and then agreeing with the advice from the API review group that nullable actually makes sense.

I don't understand why nullable makes sense here. I was away for the API review, and see the comments in the issue:

Changed the HttpRequestError property on HttpRequestException to be nullable, to distinguish "the provider didn't give one" from "the provider had an error it couldn't map".

but I agree that this is a confusing distinction to try to make. What action is someone supposed to take differently based on null vs Unknown? You get an exception, and if an error code couldn't be produced, for whatever reason, it makes sense that it's "unknown". I don't know what I'd do differently between a null error code and an Unknown error code other than have to write more code and be confused.

cc: @bartonjs, @terrajobst

The main distinction that I can think of would be "I should try again if the error is [some specific code(s)], and not if not, and Unknown is clearly not one of those codes". If it's null (vs Unknown) then the caller can decide "the underlying provider doesn't know how to set this code" is different from "something went wrong that the caller didn't know how to map to this enum"

I still don't understand the distinction / how someone would apply it. You're saying someone would write code that would retry on null but not on Unknown, or retry on Unknown but not null? Whether the provider knows what the problem is but can't map it, or doesn't know and can't map it, the result is the same: the error is unknown and you as the consumer have zero additional information to inform you as to what to do with it. I'm not understanding why we'd choose two different representations for that same case.

If there's an additional error condition that can't be mapped to one in our enum, I'd think the right answer would be to augment the enum with the additional values for those conditions.

src/libraries/System.Net.Http/src/System/Net/Http/HttpIOException.cs

src/libraries/Common/tests/System/Net/Http/ResponseStreamTest.cs

antonfirsov · 2023-07-17T18:11:36Z

/azp run runtime-extra-platforms

azure-pipelines · 2023-07-17T18:11:49Z

Azure Pipelines successfully started running 1 pipeline(s).

antonfirsov · 2023-07-17T18:12:41Z

/azp run runtime-libraries-coreclr outerloop

azure-pipelines · 2023-07-17T18:12:49Z

Azure Pipelines successfully started running 1 pipeline(s).

wfurt · 2023-07-17T20:06:11Z

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs

+
+            static HttpRequestError DeduceError(Exception exception)
+            {
+                // TODO: Deduce quic errors from QuicException.TransportErrorCode once https://github.com/dotnet/runtime/issues/87262 is implemented.


87262 is merged. can be updated later.

wfurt

looks good enough for me. I think we can resolve outstanding issues as follow-up

wfurt · 2023-07-17T20:14:49Z

it seems ike test failures may be related - for other platforms. We should ideally resolve before merging @antonfirsov. (or at least disable so we do not pollute CI)

MihaZupan

Left a few comments, but SETTINGS_ENABLE_CONNECT_PROTOCOL is the only one that should be resolved before merging.

src/libraries/System.Net.Http/src/System/Net/Http/HttpRequestError.cs

MihaZupan · 2023-07-17T20:16:46Z

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs

+                        var innerException = HttpProtocolException.CreateHttp3StreamException(code, ex);
+                        throw new HttpRequestException(SR.net_http_client_execution_error, innerException, httpRequestError: HttpRequestError.HttpProtocolError);


Was dropping the AbortException here intentional?

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs

src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionPool.cs

MihaZupan · 2023-07-17T20:28:12Z

src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/WebSocketHandle.Managed.cs

@@ -135,7 +135,7 @@ public async Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, Cancellatio
                        break;
                    }
                    catch (HttpRequestException ex) when
-                        ((ex.Data.Contains("SETTINGS_ENABLE_CONNECT_PROTOCOL") || ex.Data.Contains("HTTP2_ENABLED"))
+                        ((ex.HttpRequestError == HttpRequestError.ExtendedConnectNotSupported || ex.Data.Contains("HTTP2_ENABLED"))


Suggested change

((ex.HttpRequestError == HttpRequestError.ExtendedConnectNotSupported || ex.Data.Contains("HTTP2_ENABLED"))

((ex.HttpRequestError is HttpRequestError.ExtendedConnectNotSupported or HttpRequestError.VersionNegotiationError)

We should be able to rely on the error code but keep the HTTP2_ENABLED in the code for backward compat.

Shouldn't it also include SecureConnectionError

Suggested change

((ex.HttpRequestError == HttpRequestError.ExtendedConnectNotSupported || ex.Data.Contains("HTTP2_ENABLED"))

((ex.HttpRequestError is HttpRequestError.ExtendedConnectNotSupported or HttpRequestError.VersionNegotiationError or HttpRequestError.SecureConnectionError)

given that this what we detect in ConnectHelper.EstablishSslConnectionAsync:

https://github.com/dotnet/runtime/blob/633d632fb5178245964b41d485cf6f64683657c1/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectHelper.cs#L92C18-L98

Or would that also catch TLS errors we don't want to catch here?

Looks like there's an overlap with #75399.

If you specify Version: 2.0, Policy: Exact and the server doesn't support HTTP/2, you will get a SecureConnectionError error, not a VersionNegotiationError with this change.

For H2 WebSockets, we would want to handle that case, but only if HTTP/2 was the problem, not for any TLS error.

I guess we can keep it as-is for now (checking HTTP2_ENABLED) and then decide if we can resolve #75399 in a way that makes using the data unnecessary.

antonfirsov · 2023-07-17T22:08:54Z

/azp run runtime-libraries-coreclr outerloop

azure-pipelines · 2023-07-17T22:09:05Z

Azure Pipelines successfully started running 1 pipeline(s).

Respond to dotnet/runtime#88974

[main] Update dependencies from dotnet/runtime - Fix tests to expect new error message. Respond to dotnet/runtime#88974

…/efcore (#49554) * Update dependencies from https://github.com/dotnet/runtime build 20230720.4 Microsoft.Bcl.AsyncInterfaces , Microsoft.Bcl.TimeProvider , Microsoft.Extensions.Caching.Abstractions , Microsoft.Extensions.Caching.Memory , Microsoft.Extensions.Configuration , Microsoft.Extensions.Configuration.Abstractions , Microsoft.Extensions.Configuration.Binder , Microsoft.Extensions.Configuration.CommandLine , Microsoft.Extensions.Configuration.EnvironmentVariables , Microsoft.Extensions.Configuration.FileExtensions , Microsoft.Extensions.Configuration.Ini , Microsoft.Extensions.Configuration.Json , Microsoft.Extensions.Configuration.UserSecrets , Microsoft.Extensions.Configuration.Xml , Microsoft.Extensions.DependencyInjection , Microsoft.Extensions.DependencyInjection.Abstractions , Microsoft.Extensions.DependencyModel , Microsoft.Extensions.Diagnostics , Microsoft.Extensions.FileProviders.Abstractions , Microsoft.Extensions.FileProviders.Composite , Microsoft.Extensions.FileProviders.Physical , Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.HostFactoryResolver.Sources , Microsoft.Extensions.Hosting , Microsoft.Extensions.Hosting.Abstractions , Microsoft.Extensions.Http , Microsoft.Extensions.Logging , Microsoft.Extensions.Logging.Abstractions , Microsoft.Extensions.Logging.Configuration , Microsoft.Extensions.Logging.Console , Microsoft.Extensions.Logging.Debug , Microsoft.Extensions.Logging.EventLog , Microsoft.Extensions.Logging.EventSource , Microsoft.Extensions.Logging.TraceSource , Microsoft.Extensions.Options , Microsoft.Extensions.Options.ConfigurationExtensions , Microsoft.Extensions.Options.DataAnnotations , Microsoft.Extensions.Primitives , Microsoft.Internal.Runtime.AspNetCore.Transport , Microsoft.NET.Runtime.MonoAOTCompiler.Task , Microsoft.NET.Runtime.WebAssembly.Sdk , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.AOT.win-x64.Cross.browser-wasm , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.BrowserDebugHost.Transport , Microsoft.NETCore.Platforms , System.Collections.Immutable , System.Composition , System.Configuration.ConfigurationManager , System.Diagnostics.DiagnosticSource , System.Diagnostics.EventLog , System.Diagnostics.PerformanceCounter , System.DirectoryServices.Protocols , System.IO.Hashing , System.IO.Pipelines , System.Net.Http.Json , System.Net.Http.WinHttpHandler , System.Reflection.Metadata , System.Resources.Extensions , System.Runtime.Caching , System.Security.Cryptography.Pkcs , System.Security.Cryptography.Xml , System.Security.Permissions , System.ServiceProcess.ServiceController , System.Text.Encodings.Web , System.Text.Json , System.Threading.AccessControl , System.Threading.Channels , System.Threading.RateLimiting From Version 8.0.0-preview.7.23367.14 -> To Version 8.0.0-preview.7.23370.4 * Update dependencies from https://github.com/dotnet/efcore build 20230720.3 dotnet-ef , Microsoft.EntityFrameworkCore , Microsoft.EntityFrameworkCore.Design , Microsoft.EntityFrameworkCore.InMemory , Microsoft.EntityFrameworkCore.Relational , Microsoft.EntityFrameworkCore.Sqlite , Microsoft.EntityFrameworkCore.SqlServer , Microsoft.EntityFrameworkCore.Tools From Version 8.0.0-preview.7.23368.5 -> To Version 8.0.0-preview.7.23370.3 * Update dependencies from https://github.com/dotnet/runtime build 20230720.6 Microsoft.Bcl.AsyncInterfaces , Microsoft.Bcl.TimeProvider , Microsoft.Extensions.Caching.Abstractions , Microsoft.Extensions.Caching.Memory , Microsoft.Extensions.Configuration , Microsoft.Extensions.Configuration.Abstractions , Microsoft.Extensions.Configuration.Binder , Microsoft.Extensions.Configuration.CommandLine , Microsoft.Extensions.Configuration.EnvironmentVariables , Microsoft.Extensions.Configuration.FileExtensions , Microsoft.Extensions.Configuration.Ini , Microsoft.Extensions.Configuration.Json , Microsoft.Extensions.Configuration.UserSecrets , Microsoft.Extensions.Configuration.Xml , Microsoft.Extensions.DependencyInjection , Microsoft.Extensions.DependencyInjection.Abstractions , Microsoft.Extensions.DependencyModel , Microsoft.Extensions.Diagnostics , Microsoft.Extensions.FileProviders.Abstractions , Microsoft.Extensions.FileProviders.Composite , Microsoft.Extensions.FileProviders.Physical , Microsoft.Extensions.FileSystemGlobbing , Microsoft.Extensions.HostFactoryResolver.Sources , Microsoft.Extensions.Hosting , Microsoft.Extensions.Hosting.Abstractions , Microsoft.Extensions.Http , Microsoft.Extensions.Logging , Microsoft.Extensions.Logging.Abstractions , Microsoft.Extensions.Logging.Configuration , Microsoft.Extensions.Logging.Console , Microsoft.Extensions.Logging.Debug , Microsoft.Extensions.Logging.EventLog , Microsoft.Extensions.Logging.EventSource , Microsoft.Extensions.Logging.TraceSource , Microsoft.Extensions.Options , Microsoft.Extensions.Options.ConfigurationExtensions , Microsoft.Extensions.Options.DataAnnotations , Microsoft.Extensions.Primitives , Microsoft.Internal.Runtime.AspNetCore.Transport , Microsoft.NET.Runtime.MonoAOTCompiler.Task , Microsoft.NET.Runtime.WebAssembly.Sdk , Microsoft.NETCore.App.Ref , Microsoft.NETCore.App.Runtime.AOT.win-x64.Cross.browser-wasm , Microsoft.NETCore.App.Runtime.win-x64 , Microsoft.NETCore.BrowserDebugHost.Transport , Microsoft.NETCore.Platforms , System.Collections.Immutable , System.Composition , System.Configuration.ConfigurationManager , System.Diagnostics.DiagnosticSource , System.Diagnostics.EventLog , System.Diagnostics.PerformanceCounter , System.DirectoryServices.Protocols , System.IO.Hashing , System.IO.Pipelines , System.Net.Http.Json , System.Net.Http.WinHttpHandler , System.Reflection.Metadata , System.Resources.Extensions , System.Runtime.Caching , System.Security.Cryptography.Pkcs , System.Security.Cryptography.Xml , System.Security.Permissions , System.ServiceProcess.ServiceController , System.Text.Encodings.Web , System.Text.Json , System.Threading.AccessControl , System.Threading.Channels , System.Threading.RateLimiting From Version 8.0.0-preview.7.23367.14 -> To Version 8.0.0-preview.7.23370.6 * Update dependencies from https://github.com/dotnet/efcore build 20230721.2 dotnet-ef , Microsoft.EntityFrameworkCore , Microsoft.EntityFrameworkCore.Design , Microsoft.EntityFrameworkCore.InMemory , Microsoft.EntityFrameworkCore.Relational , Microsoft.EntityFrameworkCore.Sqlite , Microsoft.EntityFrameworkCore.SqlServer , Microsoft.EntityFrameworkCore.Tools From Version 8.0.0-preview.7.23368.5 -> To Version 8.0.0-preview.7.23371.2 * Fix tests to expect new error message. Respond to dotnet/runtime#88974 --------- Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Eric Erhardt <[email protected]>

antonfirsov added 2 commits July 17, 2023 03:00

implement HttpRequestError

8cad616

DisableParallelization for http3 tests

511e16c

antonfirsov added the area-System.Net.Http label Jul 17, 2023

antonfirsov added this to the 8.0.0 milestone Jul 17, 2023

antonfirsov requested review from MihaZupan and a team July 17, 2023 01:08

dotnet-issue-labeler bot added the new-api-needs-documentation label Jul 17, 2023

ghost assigned antonfirsov Jul 17, 2023

antonfirsov commented Jul 17, 2023

View reviewed changes

This was referenced Jul 17, 2023

LibraryImportGenerator.Unit.Tests crashed in CI #87951

Closed

System.Net.Http.Functional.Tests.HttpMetricsTest_Http11_Async.FailedRequests_ConnectionClosedWhileReceivingHeaders_Recorded timed out #88868

Closed

MihaZupan reviewed Jul 17, 2023

View reviewed changes

ManickaP reviewed Jul 17, 2023

View reviewed changes

build-analysis bot mentioned this pull request Jul 17, 2023

timeout in FailedRequests_ConnectionClosedWhileReceivingHeaders_Recorded #88921

Closed

stephentoub reviewed Jul 17, 2023

View reviewed changes

src/libraries/System.Net.Http/src/System/Net/Http/HttpIOException.cs Outdated Show resolved Hide resolved

stephentoub reviewed Jul 17, 2023

View reviewed changes

src/libraries/System.Net.Http/src/System/Net/Http/HttpIOException.cs Show resolved Hide resolved

antonfirsov added 5 commits July 17, 2023 18:33

review suggestions

12e67e7

revert deletion of HTTP2_ENABLED

633d632

cleanup some HTTP/3 cases

9afb790

test fixes

c7ac73b

override HttpIOException.Message to include HttpRequestError

2eb3a4a

wfurt reviewed Jul 17, 2023

View reviewed changes

src/libraries/Common/tests/System/Net/Http/ResponseStreamTest.cs Outdated Show resolved Hide resolved

dotnet deleted a comment from azure-pipelines bot Jul 17, 2023

wfurt reviewed Jul 17, 2023

View reviewed changes

wfurt approved these changes Jul 17, 2023

View reviewed changes

MihaZupan approved these changes Jul 17, 2023

View reviewed changes

fix http2 websocket test

24dd331

This was referenced Jul 17, 2023

The given key 'size' was not present in the dictionary in coreclr libraries outerloop tests #87359

Closed

Timeout on System.Formats.Tar.Tests.TarWriter_WriteEntry_LongFile_Tests.WriteEntry_LongFileSize #88271

Closed

review findings

b54e8c7

antonfirsov merged commit 04bb7e5 into dotnet:main Jul 18, 2023

antonfirsov mentioned this pull request Jul 18, 2023

[HttpRequestError] Handle HTTP/3 cases better if needed #89097

Closed

MihaZupan mentioned this pull request Jul 21, 2023

[release/8.0-preview7] Update dependencies from dotnet/runtime dotnet/efcore dotnet/aspnetcore#49554

Merged

eerhardt mentioned this pull request Jul 21, 2023

[main] Update dependencies from dotnet/runtime dotnet/aspnetcore#49567

Merged

eerhardt added a commit to dotnet/aspnetcore that referenced this pull request Jul 21, 2023

Fix tests to expect new error message.

5772938

Respond to dotnet/runtime#88974

eerhardt added a commit to dotnet/aspnetcore that referenced this pull request Jul 21, 2023

Fix tests to expect new error message.

5aec22d

Respond to dotnet/runtime#88974

eerhardt added a commit to dotnet/aspnetcore that referenced this pull request Jul 21, 2023

Fix tests to expect new error message.

3037617

Respond to dotnet/runtime#88974

dotnet-maestro bot added a commit to dotnet/aspnetcore that referenced this pull request Jul 21, 2023

[main] Update dependencies from dotnet/runtime (#49567)

11083b2

[main] Update dependencies from dotnet/runtime - Fix tests to expect new error message. Respond to dotnet/runtime#88974

ghost locked as resolved and limited conversation to collaborators Aug 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement HttpRequestError #88974

Implement HttpRequestError #88974

antonfirsov commented Jul 17, 2023

dotnet-issue-labeler bot commented Jul 17, 2023

ghost commented Jul 17, 2023

antonfirsov Jul 17, 2023

ManickaP Jul 17, 2023

antonfirsov Jul 17, 2023 •

edited

Loading

MihaZupan Jul 17, 2023

antonfirsov Jul 17, 2023

ManickaP Jul 17, 2023

antonfirsov Jul 17, 2023 •

edited

Loading

stephentoub Jul 17, 2023 •

edited

Loading

bartonjs Jul 17, 2023

stephentoub Jul 17, 2023 •

edited

Loading

antonfirsov commented Jul 17, 2023

azure-pipelines bot commented Jul 17, 2023

antonfirsov commented Jul 17, 2023

azure-pipelines bot commented Jul 17, 2023

wfurt Jul 17, 2023

wfurt left a comment

wfurt commented Jul 17, 2023

MihaZupan left a comment

MihaZupan Jul 17, 2023

MihaZupan Jul 17, 2023 •

edited

Loading

antonfirsov Jul 17, 2023

MihaZupan Jul 17, 2023

antonfirsov commented Jul 17, 2023

azure-pipelines bot commented Jul 17, 2023

	private static void ThrowRetry(string message, Exception? innerException = null) =>
	throw new HttpRequestException(message, innerException, allowRetry: RequestRetryType.RetryOnConnectionFailure);

	catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted && _connection.AbortException != null)
	{
	// we close the connection, propagate the AbortException
	throw new HttpRequestException(SR.net_http_client_execution_error, _connection.AbortException, httpRequestError: HttpRequestError.Unknown);
	}

	else
	{
	Debug.Assert(_requestBodyCancellationSource.IsCancellationRequested);
	throw new HttpRequestException(SR.net_http_request_aborted, ex, RequestRetryType.RetryOnConnectionFailure, httpRequestError: HttpRequestError.Unknown);
	}

	catch (Exception ex)
	{
	_stream.Abort(QuicAbortDirection.Write, (long)Http3ErrorCode.InternalError);
	if (ex is HttpRequestException)
	{
	throw;
	}
	throw new HttpRequestException(SR.net_http_client_execution_error, ex, httpRequestError: HttpRequestError.Unknown);

		var innerException = HttpProtocolException.CreateHttp3StreamException(code, ex);
		throw new HttpRequestException(SR.net_http_client_execution_error, innerException, httpRequestError: HttpRequestError.HttpProtocolError);

	((ex.HttpRequestError == HttpRequestError.ExtendedConnectNotSupported \|\| ex.Data.Contains("HTTP2_ENABLED"))
	((ex.HttpRequestError is HttpRequestError.ExtendedConnectNotSupported or HttpRequestError.VersionNegotiationError)

Implement HttpRequestError #88974

Implement HttpRequestError #88974

Conversation

antonfirsov commented Jul 17, 2023

dotnet-issue-labeler bot commented Jul 17, 2023

ghost commented Jul 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antonfirsov Jul 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antonfirsov Jul 17, 2023 • edited Loading

Choose a reason for hiding this comment

stephentoub Jul 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stephentoub Jul 17, 2023 • edited Loading

Choose a reason for hiding this comment

antonfirsov commented Jul 17, 2023

azure-pipelines bot commented Jul 17, 2023

antonfirsov commented Jul 17, 2023

azure-pipelines bot commented Jul 17, 2023

Choose a reason for hiding this comment

wfurt left a comment

Choose a reason for hiding this comment

wfurt commented Jul 17, 2023

MihaZupan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MihaZupan Jul 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antonfirsov commented Jul 17, 2023

azure-pipelines bot commented Jul 17, 2023

antonfirsov Jul 17, 2023 •

edited

Loading

antonfirsov Jul 17, 2023 •

edited

Loading

stephentoub Jul 17, 2023 •

edited

Loading

stephentoub Jul 17, 2023 •

edited

Loading

MihaZupan Jul 17, 2023 •

edited

Loading