-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Forward StatusCode to HttpRequestException whenever possible #116257
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: MihaZupan <[email protected]>
|
Tagging subscribers to this area: @dotnet/ncl |
.../System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs
Show resolved
Hide resolved
Co-authored-by: stephentoub <[email protected]>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. |
|
Are these the only 3 cases of the status code not being propagated to exceptions? |
After conducting a comprehensive search through the System.Net.Http codebase, these appear to be the only 3 cases where status codes weren't being propagated to HttpRequestException when they should have been. I systematically examined all files that throw HttpRequestException and checked for:
The other similar patterns I found are already handled correctly:
So the 3 locations identified in issue #105700 appear to be the complete set of missing cases. |
antonfirsov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that when I opened #105700, I was just looking at code patterns without investing time to analyze how realistic it is to hit actual practical cases of missing a status code that could be present.
This is only a good issue for copilot if we are fine fixing the code so it has the right pattern, but without looking deeper and trying to understand/test the scenarios.
.../System.Net.Http/src/System/Net/Http/SocketsHttpHandler/ConnectionPool/HttpConnectionPool.cs
Show resolved
Hide resolved
| }); | ||
| } | ||
| catch | ||
| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leaving that catch block completely empty seems wrong; since this should trigger an error, shouldn't we assert that we got here?
|
Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it. |
This PR forwards HTTP status codes to
HttpRequestExceptionin three locations where they were previously not being included, following up on #105610 & #105546.Changes Made
HttpConnectionPool.cs (line 522): When throwing
VersionNegotiationErrordue to version policy restrictions, now forwards theStatusCodefrom the innerHttpRequestExceptionthat triggered the retry.HttpConnection.cs (line 2080): When throwing
UserAuthenticationErrordue to connection close during response draining, now forwards theStatusCodefrom theHttpResponseMessage.HttpConnection.cs (line 2096): When throwing
UserAuthenticationErrordue to failed response draining, now forwards theStatusCodefrom theHttpResponseMessage.Example
Before this change, these exceptions would not include status code information:
After this change, status codes are properly forwarded:
This provides better diagnostic information to callers who can now access the HTTP status code that caused the underlying failure.
Testing
Fixes #105700.
Warning
Firewall rules blocked me from connecting to one or more addresses
I tried to connect to the following addresses, but was blocked by firewall rules:
badhost/home/REDACTED/work/runtime/runtime/artifacts/bin/testhost/net10.0-linux-Debug-x64/dotnet exec --runtimeconfig System.Net.Http.Functional.Tests.runtimeconfig.json --depsfile System.Net.Http.Functional.Tests.deps.json /home/REDACTED/.nuget/packages/microsoft.dotnet.xunitconsoleREDACTED/2.9.2-beta.25260.104/build/../tools/net/xunit.console.dll System.Net.Http.Functional.Tests.dll -xml testResults.xml -nologo -notrait category=OuterLoop -notrait category=failing(dns block)does.not.exist.sorry/home/REDACTED/work/runtime/runtime/artifacts/bin/testhost/net10.0-linux-Debug-x64/dotnet exec --runtimeconfig /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/System.Net.Http.Functional.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/System.Net.Http.Functional.Tests.deps.json /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/Microsoft.DotNet.RemoteExecutor.dll System.Net.Http.Functional.Tests, Version=10.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51 System.Net.Http.Functional.Tests.DiagnosticsTest <SendAsync_ConnectionFailure_RecordsActivitiesWithCorrectErrorInfo>g__RunTest|18_0 /tmp/delryg3w.4s2 1.1 False dns(dns block)/home/REDACTED/work/runtime/runtime/artifacts/bin/testhost/net10.0-linux-Debug-x64/dotnet exec --runtimeconfig /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/System.Net.Http.Functional.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/System.Net.Http.Functional.Tests.deps.json /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/Microsoft.DotNet.RemoteExecutor.dll System.Net.Http.Functional.Tests, Version=10.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51 System.Net.Http.Functional.Tests.DiagnosticsTest <SendAsync_ConnectionFailure_RecordsActivitiesWithCorrectErrorInfo>g__RunTest|18_0 /tmp/3iguzv2q.i3r 1.1 True dns(dns block)/home/REDACTED/work/runtime/runtime/artifacts/bin/testhost/net10.0-linux-Debug-x64/dotnet exec --runtimeconfig /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/System.Net.Http.Functional.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/System.Net.Http.Functional.Tests.deps.json /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/Microsoft.DotNet.RemoteExecutor.dll System.Net.Http.Functional.Tests, Version=10.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51 System.Net.Http.Functional.Tests.DiagnosticsTest <SendAsync_ConnectionFailure_RecordsActivitiesWithCorrectErrorInfo>g__RunTest|18_0 /tmp/gmfbd2cq.feu 2.0 True dns(dns block)nosuchhost.invalid/home/REDACTED/work/runtime/runtime/artifacts/bin/testhost/net10.0-linux-Debug-x64/dotnet exec --runtimeconfig System.Net.Http.Functional.Tests.runtimeconfig.json --depsfile System.Net.Http.Functional.Tests.deps.json /home/REDACTED/.nuget/packages/microsoft.dotnet.xunitconsoleREDACTED/2.9.2-beta.25260.104/build/../tools/net/xunit.console.dll System.Net.Http.Functional.Tests.dll -xml testResults.xml -nologo -notrait category=OuterLoop -notrait category=failing(dns block)/home/REDACTED/work/runtime/runtime/artifacts/bin/testhost/net10.0-linux-Debug-x64/dotnet exec --runtimeconfig /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/System.Net.Http.Functional.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/System.Net.Http.Functional.Tests.deps.json /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/Microsoft.DotNet.RemoteExecutor.dll System.Net.Http.Functional.Tests, Version=10.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51 System.Net.Http.Functional.Tests.DiagnosticsTest+<>c <SendAsync_ExpectedDiagnosticExceptionActivityLogging>b__23_0 /tmp/bn1qqsgk.chn 2.0 True(dns block)/home/REDACTED/work/runtime/runtime/artifacts/bin/testhost/net10.0-linux-Debug-x64/dotnet exec --runtimeconfig /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/System.Net.Http.Functional.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/System.Net.Http.Functional.Tests.deps.json /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/Microsoft.DotNet.RemoteExecutor.dll System.Net.Http.Functional.Tests, Version=10.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51 System.Net.Http.Functional.Tests.DiagnosticsTest+<>c <SendAsync_ExpectedDiagnosticExceptionActivityLogging>b__23_0 /tmp/3nuqi3xb.doc 1.1 False(dns block)www.microsoft.com/home/REDACTED/work/runtime/runtime/artifacts/bin/testhost/net10.0-linux-Debug-x64/dotnet exec --runtimeconfig /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/System.Net.Http.Functional.Tests.runtimeconfig.json --depsfile /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/System.Net.Http.Functional.Tests.deps.json /home/REDACTED/work/runtime/runtime/artifacts/bin/System.Net.Http.Functional.Tests/Debug/net10.0-linux/Microsoft.DotNet.RemoteExecutor.dll System.Net.Http.Functional.Tests, Version=10.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51 System.Net.Http.Functional.Tests.HttpClientHandler_ServerCertificates_Test+<>c <HttpClientUsesSslCertEnvironmentVariables>b__26_0 /tmp/2tawtdzz.cvv 1.1 True(dns block)www.some.example/home/REDACTED/work/runtime/runtime/artifacts/bin/testhost/net10.0-linux-Debug-x64/dotnet exec --runtimeconfig System.Net.Http.Functional.Tests.runtimeconfig.json --depsfile System.Net.Http.Functional.Tests.deps.json /home/REDACTED/.nuget/packages/microsoft.dotnet.xunitconsoleREDACTED/2.9.2-beta.25260.104/build/../tools/net/xunit.console.dll System.Net.Http.Functional.Tests.dll -xml testResults.xml -nologo -notrait category=OuterLoop -notrait category=failing(dns block)If you need me to access, download, or install something from one of these locations, you can either:
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.