Skip to content

Preserve HTTP status codes across target frameworks - #1767

Open
PranavSenthilnathan wants to merge 1 commit into
modelcontextprotocol:mainfrom
PranavSenthilnathan:pranavsenthilnathan-portable-http-status-preservation
Open

Preserve HTTP status codes across target frameworks#1767
PranavSenthilnathan wants to merge 1 commit into
modelcontextprotocol:mainfrom
PranavSenthilnathan:pranavsenthilnathan-portable-http-status-preservation

Conversation

@PranavSenthilnathan

Copy link
Copy Markdown
Contributor

Motivation and Context

HttpRequestException.StatusCode is unavailable on netstandard2.0, so HTTP status information is currently lost on that target. AutoDetect also recreates the original Streamable HTTP exception when SSE fallback fails, which must preserve the status for downstream handling.

This prepares for #1765, where connect-time fallback needs to distinguish HTTP 400/404 responses consistently across all supported target frameworks.

Changes

  • Add internal helpers that create HttpRequestException instances while preserving the HTTP status in Exception.Data.
  • Prefer the native HttpRequestException.StatusCode where available, with the stored value as a cross-target fallback.
  • Preserve the original Streamable HTTP status when AutoDetect wraps a failed SSE fallback.
  • Add coverage for status preservation across net10.0, net9.0, net8.0, and net472.

No public API is added or changed.

How Has This Been Tested?

  • dotnet build
  • Focused AutoDetect transport tests on net10.0, net9.0, net8.0, and net472
  • Full core test suites

The ASP.NET conformance suite could not start locally because Node.js 21.6.2 does not provide fs.globSync, which is required by the pinned conformance package.

Breaking Changes

None.

This pull request description was generated with GitHub Copilot.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 9a49da98-c8d8-4b90-a6ed-50afe18ac981

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves cross-target consistency for HTTP error handling by preserving HTTP status codes on HttpRequestException even on targets that lack HttpRequestException.StatusCode (notably netstandard2.0). It does so by storing the status code in Exception.Data and providing helpers to create/read exceptions in a uniform way, including when AutoDetect wraps a failed SSE fallback.

Changes:

  • Introduce HttpRequestExceptionExtensions to create HttpRequestException instances while persisting HttpStatusCode in Exception.Data, and to retrieve the status via StatusCode when available or the stored value otherwise.
  • Update AutoDetect’s SSE-fallback wrapping path to rethrow an HttpRequestException created via the helper, preserving the original Streamable HTTP status code across all TFMs.
  • Extend AutoDetect transport tests to assert status preservation via Exception.Data (and via StatusCode when available).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
tests/ModelContextProtocol.Tests/Transport/HttpClientTransportAutoDetectTests.cs Adds assertions that surfaced HttpRequestException instances preserve the original HTTP status code via Exception.Data (and StatusCode on #if NET).
src/ModelContextProtocol.Core/Client/AutoDetectingClientSessionTransport.cs Switches the dual-failure (Streamable HTTP + SSE fallback) wrap to use the new helper and preserve status codes consistently.
src/Common/HttpResponseMessageExtensions.cs Adds internal helpers to create/read HttpRequestException with status preserved via Exception.Data as a cross-target fallback.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants