Skip to content
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

HttpClient should report protocol error code when the server closes the connection #62228

Closed
JamesNK opened this issue Dec 1, 2021 · 4 comments
Assignees
Labels
area-System.Net.Http enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@JamesNK
Copy link
Member

JamesNK commented Dec 1, 2021

Description

Noticed in #62216

Basically, the server is gracefully closing the HTTP/2 connection with protocol error of ENHANCE_YOUR_CALM. The error in the client from reading the response stream doesn't mention that error. Instead, the error is System.IO.IOException: The response ended prematurely while waiting for the next frame from the server..

The error is technically correct, but it could be more helpful if it includes information that the server decided to proactively close the connection using GO_AWAY (or abort in HTTP/3). The protocol error is a clue to why the server decided to close the connection.

@geoffkizer #62216 (comment)

Less important, but SocketsHttpHandler could provide better error details in the exception. While "The response ended prematurely while waiting for the next frame from the server." is technically correct, reporting the ENHANCE_YOUR_CALM status from the server would make it clearer what is going on.

Agreed. When the server gracefully (i.e. not in the middle of a frame) closes the connection after a GOAWAY with an error code is received, we should fail any outstanding requests with an exception that indicates the server error from GOAWAY.

This should be done in HTTP/2 and HTTP/3.

Reproduction Steps

  1. Read from content response stream.
  2. While waiting for data, the server closes the connection with a GO_AWAY frame.
  3. Catch error from response stream ReadAsync.

Expected behavior

Exception includes details about the protocol error, like SendAsync reports.

Actual behavior

IOException: The request was aborted. IOException: The response ended prematurely while waiting for the next frame from the server.", DebugException="System.IO.IOException: The request was aborted.
---> System.IO.IOException: The response ended prematurely while waiting for the next frame from the server.
at System.Net.Http.Http2Connection.g__ThrowMissingFrame|57_1()
at System.Net.Http.Http2Connection.ReadFrameAsync(Boolean initialFrame)
at System.Net.Http.Http2Connection.ProcessIncomingFramesAsync()
--- End of inner exception stack trace ---
at System.Net.Http.Http2Connection.ThrowRequestAborted(Exception innerException)
at System.Net.Http.Http2Connection.Http2Stream.CheckResponseBodyState()
at System.Net.Http.Http2Connection.Http2Stream.TryReadFromBuffer(Span1 buffer, Boolean partOfSyncRead) at System.Net.Http.Http2Connection.Http2Stream.ReadDataAsync(Memory1 buffer, HttpResponseMessage responseMessage, CancellationToken cancellationToken)
at Grpc.Net.Client.StreamExtensions.ReadMessageContent(Stream responseStream, Memory`1 messageData, Int32 length, CancellationToken cancellationToken) in /var/local/git/grpc-dotnet/src/Grpc.Net.Client/Internal/StreamExtensions.cs:line 224

Regression?

No

Known Workarounds

No response

Configuration

No response

Other information

No response

@ghost
Copy link

ghost commented Dec 1, 2021

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

Issue Details

Description

Noticed in #62216

Basically, the server is gracefully closing the HTTP/2 connection with protocol error of ENHANCE_YOUR_CALM. The error in the client doesn't mention that error. Instead, it is System.IO.IOException: The response ended prematurely while waiting for the next frame from the server.. This error is technically correct, but it could be more helpful if it includes detail that the server decided to close the connection with a specific protocol error.

@geoffkizer:

Less important, but SocketsHttpHandler could provide better error details in the exception. While "The response ended prematurely while waiting for the next frame from the server." is technically correct, reporting the ENHANCE_YOUR_CALM status from the server would make it clearer what is going on.

Agreed. When the server gracefully (i.e. not in the middle of a frame) closes the connection after a GOAWAY with an error code is received, we should fail any outstanding requests with an exception that indicates the server error from GOAWAY.

This should be done in HTTP/2 and HTTP/3.

Reproduction Steps

  1. Read from content response stream.
  2. While waiting for data, the server closes the connection with a GO_AWAY frame.
  3. Catch error from response stream ReadAsync.

Expected behavior

Exception includes details about the protocol error, like SendAsync reports.

Actual behavior

IOException: The request was aborted. IOException: The response ended prematurely while waiting for the next frame from the server.", DebugException="System.IO.IOException: The request was aborted.
---> System.IO.IOException: The response ended prematurely while waiting for the next frame from the server.
at System.Net.Http.Http2Connection.g__ThrowMissingFrame|57_1()
at System.Net.Http.Http2Connection.ReadFrameAsync(Boolean initialFrame)
at System.Net.Http.Http2Connection.ProcessIncomingFramesAsync()
--- End of inner exception stack trace ---
at System.Net.Http.Http2Connection.ThrowRequestAborted(Exception innerException)
at System.Net.Http.Http2Connection.Http2Stream.CheckResponseBodyState()
at System.Net.Http.Http2Connection.Http2Stream.TryReadFromBuffer(Span1 buffer, Boolean partOfSyncRead) at System.Net.Http.Http2Connection.Http2Stream.ReadDataAsync(Memory1 buffer, HttpResponseMessage responseMessage, CancellationToken cancellationToken)
at Grpc.Net.Client.StreamExtensions.ReadMessageContent(Stream responseStream, Memory`1 messageData, Int32 length, CancellationToken cancellationToken) in /var/local/git/grpc-dotnet/src/Grpc.Net.Client/Internal/StreamExtensions.cs:line 224

Regression?

No

Known Workarounds

No response

Configuration

No response

Other information

No response

Author: JamesNK
Assignees: -
Labels:

area-System.Net.Http

Milestone: -

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Dec 1, 2021
@karelz karelz added enhancement Product code improvement that does NOT require public API changes/additions and removed untriaged New issue has not been triaged by the area owner labels Dec 2, 2021
@karelz karelz added this to the 7.0.0 milestone Dec 2, 2021
@karelz
Copy link
Member

karelz commented Dec 2, 2021

Triage:

  • Would be nice to improve diagnostics. If easy, let's do it in 7.0.

@antonfirsov
Copy link
Member

antonfirsov commented May 3, 2022

@JamesNK is this about a more verbose exception message, or do you expect us to report the protocol error through some observable property? (Eg in an extra property in a derived HttpIOException)

@antonfirsov
Copy link
Member

Closing this in favor of #70684

@ghost ghost locked as resolved and limited conversation to collaborators Jul 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Net.Http enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

3 participants