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.

Sign up for GitHub

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

Jump to bottom

http2: The client closing the connection is not an error #4284

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

http2: The client closing the connection is not an error #4284

wants to merge 3 commits into from

Conversation

nigoroll
Copy link
Member

It is fine for a client to close the connection, we should say a friendly goodbye and log this as no error.

Closes #4281
Related to #4283

@nigoroll
Copy link
Member Author

good feedback from bugwash:

  • differentiate three cases, if only for reporting: eof during rx frame / between frames but streams open / idle
  • differentiate goaway seen from client
  • re-check rfc9113

@nigoroll nigoroll marked this pull request as draft February 24, 2025 14:14
@nigoroll nigoroll mentioned this pull request Feb 24, 2025
Open
@nigoroll nigoroll marked this pull request as ready for review February 24, 2025 20:29
@nigoroll
Copy link
Member Author

So, I did my homework:
I maintain that NO_ERROR and SessClose REM_CLOSE is the (only) right thing to log if the client has closed. The GOAWAY review created many more TODOs, but none in this area. The client can just close its connection, and that's fine. We should actually (try to) finish sending any data from open streams, but we can not read any data. But that "finish up" is for later (#4285) for now, this PR is to fix an error message.

@nigoroll nigoroll force-pushed the h2_eof_goaway_no_error branch from cac7eb3 to 51fa079 Compare February 24, 2025 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Confusing behavior for H2 session closed by client before a request is sent
1 participant
@nigoroll