Skip to content

Throw HttpSysException with HttpInitialize status details#66859

Open
Copilot wants to merge 7 commits into
mainfrom
copilot/add-httpinitialize-hresult-exception-details
Open

Throw HttpSysException with HttpInitialize status details#66859
Copilot wants to merge 7 commits into
mainfrom
copilot/add-httpinitialize-hresult-exception-details

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 26, 2026

Throw HttpSysException with HttpInitialize status details

Surface actionable diagnostics when HttpInitialize fails during Http.Sys startup.

Description

  • Problem
    • HttpSysListener previously threw a bare PlatformNotSupportedException when HttpApi.Supported was false, hiding native failure details.
  • Implementation
    • Added HttpApi.HttpInitializeStatusCode to persist the native HttpInitialize return value.
    • Replaced the bare throw in HttpSysListener with CreateHttpInitializeFailureException(...).
    • For non-success status codes, the listener now throws HttpSysException directly (instead of wrapping it).
    • Exception message includes the Win32 status code (0xXXXXXXXX).
    • Preserved PlatformNotSupportedException fallback when status is ERROR_SUCCESS (no specific native error to surface).
    • Removed the previously added HRESULT text and related helper logic based on feedback.
  • Coverage
    • Added/updated focused unit tests for:
      • non-success status code => returned exception is HttpSysException with status details
      • success status code => fallback PlatformNotSupportedException without inner exception
if (!HttpApi.Supported)
{
    throw CreateHttpInitializeFailureException(HttpApi.HttpInitializeStatusCode);
}

Customer Impact

Http.Sys startup failures now provide immediate root-cause diagnostics with native error semantics (HttpSysException + status code), reducing investigation time for unsupported/failed initialization environments.

Regression?

  • Yes
  • No

[If yes, specify the version the behavior has regressed from]

Risk

  • High
  • Medium
  • Low

Error-path exception enrichment and exception-type refinement only; no request-processing behavior changes.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

When servicing release/2.3

  • Make necessary changes in eng/PatchConfig.props

Copilot AI and others added 2 commits May 26, 2026 21:05
Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
Copilot AI changed the title [WIP] Add HttpInitialize HRESULT to HttpSysListener exception details Include HttpInitialize status/HRESULT in HttpSysListener unsupported exception May 26, 2026
Copilot AI requested a review from BrennanConroy May 26, 2026 21:09
}

var httpInitializeException = new Win32Exception((int)httpInitializeStatusCode);
return new PlatformNotSupportedException(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe should throw an HttpSysException


internal static PlatformNotSupportedException CreateHttpInitializeFailureException(uint httpInitializeStatusCode)
{
if (httpInitializeStatusCode == ErrorCodes.ERROR_SUCCESS)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Comment here?

@github-actions github-actions Bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label May 26, 2026
@BrennanConroy
Copy link
Copy Markdown
Member

@copilot react to feedback

Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 26, 2026

@copilot react to feedback

Addressed in edcf167. I updated the exception path to use HttpSysException as the inner exception and added a clarifying comment for the success-code fallback branch, then updated the focused tests accordingly.

}

var httpInitializeException = new HttpSysException((int)httpInitializeStatusCode);
return new PlatformNotSupportedException(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wonder if this should just throw the HttpSysException? Any reason to keep using PlatformNotSupportedException?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot see above

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in 6b01d35: for non-success HttpInitialize status codes, HttpSysListener now throws HttpSysException directly (with status code + HRESULT in the message). The ERROR_SUCCESS branch still falls back to PlatformNotSupportedException since there is no native failure code to surface.

Copilot AI and others added 2 commits May 26, 2026 21:29
Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
Copilot AI changed the title Include HttpInitialize status/HRESULT in HttpSysListener unsupported exception Throw HttpSysException with HttpInitialize status/HRESULT details May 26, 2026
Copilot AI requested a review from BrennanConroy May 26, 2026 21:34
var hResult = CreateHResultFromWin32Error(httpInitializeStatusCode);
return new HttpSysException(
(int)httpInitializeStatusCode,
$"HttpInitialize failed with status code 0x{httpInitializeStatusCode:X8} (HRESULT 0x{hResult:X8}).");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@copilot I don't think the HResult part is needed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in 502f04ff: removed the HRESULT text from the HttpInitialize failure message and updated the related HttpSysListenerTests assertions to only validate the status code and exception type/error code.

Co-authored-by: BrennanConroy <7574801+BrennanConroy@users.noreply.github.com>
@BrennanConroy BrennanConroy marked this pull request as ready for review May 26, 2026 21:56
@BrennanConroy BrennanConroy requested a review from halter73 as a code owner May 26, 2026 21:56
Copilot AI review requested due to automatic review settings May 26, 2026 21:56
Copilot AI changed the title Throw HttpSysException with HttpInitialize status/HRESULT details Throw HttpSysException with HttpInitialize status details May 26, 2026
Copilot AI requested a review from BrennanConroy May 26, 2026 21:56
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 Http.Sys startup diagnostics by preserving and surfacing the native HttpInitialize return status when HttpApi.Supported is false, so failures provide actionable error information instead of a generic PlatformNotSupportedException.

Changes:

  • Added HttpApi.HttpInitializeStatusCode to retain the HttpInitialize return value from HttpApi static initialization.
  • Updated HttpSysListener to throw a tailored exception via CreateHttpInitializeFailureException(...) when Http.Sys initialization is not supported.
  • Added unit tests validating exception type/contents for both success and non-success status codes.

Reviewed changes

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

File Description
src/Servers/HttpSys/test/Tests/HttpSysListenerTests.cs Adds unit tests for the new initialization-failure exception behavior.
src/Servers/HttpSys/src/NativeInterop/HttpApi.cs Persists HttpInitialize status code for later diagnostics.
src/Servers/HttpSys/src/HttpSysListener.cs Throws a diagnostic exception (including status code) when HttpInitialize support is unavailable.

Comment on lines +11 to +16
public void CreateHttpInitializeFailureException_WithErrorCode_ReturnsHttpSysExceptionWithDetails()
{
var errorCode = ErrorCodes.ERROR_ACCESS_DENIED;
var exception = Assert.IsType<HttpSysException>(HttpSysListener.CreateHttpInitializeFailureException(errorCode));

Assert.Contains($"status code 0x{errorCode:X8}", exception.Message);
Comment on lines +267 to +269
return new HttpSysException(
(int)httpInitializeStatusCode,
$"HttpInitialize failed with status code 0x{httpInitializeStatusCode:X8}.");
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add HttpInitialize HRESULT to HttpSysListener exception details

3 participants