Skip to content

Commit 93d49b4

Browse files
authored
Propagate _abortException in Http2Connection.SetupAsync (#119022) (#124639)
## Summary - Propagate `_abortException` instead of `ObjectDisposedException` in `Http2Connection.SetupAsync` catch block, surfacing the actual failure reason (e.g., server-initiated disconnect) instead of a generic ODE - Add test verifying that server disconnect during HTTP/2 setup propagates a meaningful exception ## Details When `ProcessIncomingFramesAsync` detects a server-initiated disconnect and calls `Abort()`, the original failure reason is stored in `_abortException`. However, `SetupAsync`'s catch block (line 258) was wrapping the uninformative `ObjectDisposedException` instead of the actual cause. This change uses `_abortException ?? e` as the inner exception, consistent with the existing pattern in `PerformWriteAsync` (line 1192) and `AddStream` (line 1598). ## Test plan - [x] New test: `ServerDisconnectDuringSetup_PropagatesMeaningfulException` - [x] All existing Http2 tests pass (858 passed, 9 skipped, 0 failed) Fixes #119022
1 parent 23bc7e3 commit 93d49b4

File tree

2 files changed

+46
-2
lines changed

2 files changed

+46
-2
lines changed

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -255,8 +255,10 @@ public async ValueTask SetupAsync(CancellationToken cancellationToken)
255255
throw;
256256
}
257257

258-
// TODO: Review this case!
259-
throw new IOException(SR.net_http_http2_connection_not_established, e);
258+
// Use _abortException if available, as it contains the real reason for the connection failure.
259+
// For example, when ProcessIncomingFramesAsync detects a server-initiated disconnect and calls Abort(),
260+
// _abortException will have the original IOException, while 'e' here may be an uninformative ObjectDisposedException.
261+
throw new IOException(SR.net_http_http2_connection_not_established, _abortException ?? e);
260262
}
261263

262264
// Avoid capturing the initial request's ExecutionContext for the entire lifetime of the new connection.

src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -687,6 +687,48 @@ public async Task GetAsync_ServerSendSettingsWithoutMaxConcurrentStreams_ClientA
687687
}
688688
}
689689

690+
[ConditionalTheory(nameof(SupportsAlpn))]
691+
[InlineData(false)] // server disconnects without sending SETTINGS
692+
[InlineData(true)] // server sends GOAWAY instead of SETTINGS
693+
public async Task ServerDisconnectDuringSetup_PropagatesMeaningfulException(bool sendGoAway)
694+
{
695+
using (Http2LoopbackServer server = Http2LoopbackServer.CreateServer())
696+
using (HttpClient client = CreateHttpClient())
697+
{
698+
Task<HttpResponseMessage> sendTask = client.GetAsync(server.Address);
699+
700+
// Accept connection and read client preface, but do NOT send SETTINGS.
701+
Http2LoopbackConnection connection = await server.AcceptConnectionAsync();
702+
703+
if (sendGoAway)
704+
{
705+
// Send GOAWAY instead of SETTINGS, then shut down.
706+
await connection.SendGoAway(0, ProtocolErrors.ENHANCE_YOUR_CALM);
707+
await connection.ShutdownSendAsync();
708+
709+
// The client should throw HttpRequestException(HttpProtocolError) wrapping
710+
// HttpProtocolException with the GOAWAY error code.
711+
await AssertProtocolErrorAsync(sendTask, ProtocolErrors.ENHANCE_YOUR_CALM);
712+
}
713+
else
714+
{
715+
// Immediately shut down the connection without sending SETTINGS.
716+
// This simulates a server-side disconnect during HTTP/2 setup.
717+
await connection.ShutdownSendAsync();
718+
719+
// The client should throw HttpRequestException(InvalidResponse) wrapping
720+
// HttpIOException(InvalidResponse) -> HttpIOException(ResponseEnded),
721+
// indicating the server disconnected before sending SETTINGS.
722+
HttpRequestException ex = await Assert.ThrowsAsync<HttpRequestException>(() => sendTask);
723+
Assert.Equal(HttpRequestError.InvalidResponse, ex.HttpRequestError);
724+
HttpIOException httpIoEx = Assert.IsAssignableFrom<HttpIOException>(ex.InnerException);
725+
Assert.Equal(HttpRequestError.InvalidResponse, httpIoEx.HttpRequestError);
726+
HttpIOException innerHttpIoEx = Assert.IsAssignableFrom<HttpIOException>(httpIoEx.InnerException);
727+
Assert.Equal(HttpRequestError.ResponseEnded, innerHttpIoEx.HttpRequestError);
728+
}
729+
}
730+
}
731+
690732
// This test is based on RFC 7540 section 6.1:
691733
// "If a DATA frame is received whose stream identifier field is 0x0, the recipient MUST
692734
// respond with a connection error (Section 5.4.1) of type PROTOCOL_ERROR."

0 commit comments

Comments
 (0)