Skip to content

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 3, 2025

This PR forwards HTTP status codes to HttpRequestException in three locations where they were previously not being included, following up on #105610 & #105546.

Changes Made

  1. HttpConnectionPool.cs (line 522): When throwing VersionNegotiationError due to version policy restrictions, now forwards the StatusCode from the inner HttpRequestException that triggered the retry.

  2. HttpConnection.cs (line 2080): When throwing UserAuthenticationError due to connection close during response draining, now forwards the StatusCode from the HttpResponseMessage.

  3. HttpConnection.cs (line 2096): When throwing UserAuthenticationError due to failed response draining, now forwards the StatusCode from the HttpResponseMessage.

Example

Before this change, these exceptions would not include status code information:

// Version negotiation error - no status code preserved
throw new HttpRequestException(HttpRequestError.VersionNegotiationError, message, innerException);

// Authentication error - no status code from response
throw new HttpRequestException(HttpRequestError.UserAuthenticationError, message);

After this change, status codes are properly forwarded:

// Version negotiation error - preserves status code from inner exception
throw new HttpRequestException(HttpRequestError.VersionNegotiationError, message, innerException, innerException.StatusCode);

// Authentication error - includes status code from response
throw new HttpRequestException(HttpRequestError.UserAuthenticationError, message, statusCode: response.StatusCode);

This provides better diagnostic information to callers who can now access the HTTP status code that caused the underlying failure.

Testing

  • All existing System.Net.Http unit tests pass (952 test cases, 0 failures)
  • Builds successfully with no warnings or errors
  • Manual verification confirms status codes are properly forwarded

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
    • Triggering command: /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
    • Triggering command: /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)
    • Triggering command: /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)
    • Triggering command: /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
    • Triggering command: /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)
    • Triggering command: /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)
    • Triggering command: /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
    • Triggering command: /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
    • Triggering command: /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.

@dotnet-policy-service dotnet-policy-service bot added the linkable-framework Issues associated with delivering a linker friendly framework label Jun 3, 2025
Co-authored-by: MihaZupan <25307628+MihaZupan@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Forward StatusCode to HttpRequestException whenever possible Forward StatusCode to HttpRequestException whenever possible Jun 3, 2025
@Copilot Copilot AI requested a review from MihaZupan June 3, 2025 13:13
Copilot finished work on behalf of MihaZupan June 3, 2025 13:13
@MihaZupan MihaZupan added area-System.Net.Http and removed linkable-framework Issues associated with delivering a linker friendly framework labels Jun 3, 2025
@MihaZupan MihaZupan added this to the 10.0.0 milestone Jun 3, 2025
Copy link
Contributor

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

Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jun 3, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Copilot finished work on behalf of stephentoub June 3, 2025 14:11
@MihaZupan
Copy link
Member

Are these the only 3 cases of the status code not being propagated to exceptions?
The issue called out these 3, but there might be more.

Copy link
Contributor Author

Copilot AI commented Jun 3, 2025

Are these the only 3 cases of the status code not being propagated to exceptions?
The issue called out these 3, but there might be more.

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:

  1. Cases where HttpResponseMessage.StatusCode is available but not forwarded
  2. Cases where an inner HttpRequestException.StatusCode could be forwarded
  3. Catch-and-rethrow scenarios

The other similar patterns I found are already handled correctly:

  • HttpConnectionPool.cs proxy tunnel errors already forward tunnelResponse.StatusCode
  • HttpResponseMessage.EnsureSuccessStatusCode() already forwards _statusCode
  • Authentication helpers use appropriate status codes (e.g., HttpStatusCode.Unauthorized)
  • Catch-and-rethrow cases either use bare throw; or ExceptionDispatchInfo.Throw() which preserve the original exception

So the 3 locations identified in issue #105700 appear to be the complete set of missing cases.

Copy link
Member

@antonfirsov antonfirsov left a 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.

});
}
catch
{
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Forward StatusCode to HttpRequestException whenever possible
5 participants