Repository navigation
Support graceful connection close #1181
Description
RFC 7540 section 6.8 seems to indicate that traffic can continue flowing in both directions after a GOAWAY frame, with the only restriction being that the recipient can't open new streams after receiving the GOAWAY:
A GOAWAY frame might not immediately precede closing of the
connection; a receiver of a GOAWAY that has no more use for the
connection SHOULD still send a GOAWAY frame before terminating the
connection.
...
Activity on streams numbered lower or equal to the last stream
identifier might still complete successfully. The sender of a GOAWAY
frame might gracefully shut down a connection by sending a GOAWAY
frame, maintaining the connection in an "open" state until all in-
progress streams complete.
But, h2's connection state machine immediately transitions to CLOSED upon sending or receiving a GOAWAY, effectively prohibiting any further traffic. Would you be interested in a patch to fix this, or is there some disconnect I'm missing between the wording of the RFC and actual implementations?
Activity
pgjones commentedon Jan 24, 2019
I'm not familiar enough with the code to know the answer to this, but I'm happy to review a patch if you research it.
dimaqq commentedon Feb 28, 2020
Code in question: https://github.com/python-hyper/hyper-h2/blob/570dc7daa480d34bcf0676e88d241112c51b1796/h2/connection.py#L1825-L1844
TL;DR as far as I understand the RFC:
0:Cross-link: https://github.com/encode/httpx/issues/828
dimaqq commentedon Apr 1, 2020
Would it help to split the input two ways:
RECV_GOAWAYfor errorserror_code != 0RECV_GRACEFUL_SHUTDOWNorRECV_LAST_STREAM_IDforerror_code == 0?ech0-py commentedon Mar 8, 2021
+1 on this issue
Since such software like HAProxy terminates connections by sending GOAWAY first and HEADERS/DATA frames right after - the current transition states are unable to handle this.
stephenc-pace commentedon Nov 18, 2022
Any updates on this?
zanieb commentedon Nov 30, 2022
I've looked at fixing this and it seems like it'd be a lot of work. I'm very hesitant to dig into it without guidance from maintainers. It seems this is further complicated by two-stage GOAWAY handling but that does not appear to be well supported in general, for example https://mailman.nginx.org/pipermail/nginx-devel/2021-April/013956.html.
neoLsH commentedon Oct 1, 2026
Coming at this from mitmproxy, where it turns up as mitmproxy/mitmproxy#7879. A fair number of sites, or really the load balancer in front of them, send a graceful GOAWAY and then finish the response they'd already started. h2 raises
Invalid input ConnectionInputs.RECV_DATA in state ConnectionState.CLOSEDat that point and the request dies.I read back through the thread and dimaqq's summary of the semantics matches my reading of RFC 9113, so I had a go at implementing it. I've got something working locally, suite passes, added tests for the draining cases. But I'm not confident I got the design right, and since the design looks like where this stalled before, I'd rather ask than open a PR that goes nowhere.
The part I'm unsure about is that I took a shortcut. Instead of adding a
RECV_GRACEFUL_SHUTDOWNinput and a draining state like dimaqq suggested, I left the connection inCLOSEDand recorded the boundary on the state machine, then let DATA/HEADERS/WINDOW_UPDATE/RST_STREAM through the KeyError path when the stream id is at or below it. My reasoning was that it wouldn't disturb anything downstream checkingstate is CLOSED.Then I actually went and looked, and that reasoning might be backwards. fetchling has
while connection.state_machine.state is not ConnectionState.CLOSEDaround its read loop, so with my version it stops reading on a graceful GOAWAY even though h2 would accept the frames. Those consumers might genuinely need a way to tell draining apart from dead, which argues for doing it with a real state instead. I don't know this codebase's history well enough to call it.Two smaller things I'd value an opinion on.
process_inputdoesn't take a stream id today and mine needs one to apply the boundary, so stream context leaks into what the docs describe as a deliberately coarse, high-level connection machine. And on two-stage GOAWAY, keeping the smallestlast_stream_idseen made it fall out for free, but zanieb's nginx link suggests it isn't widely supported, so I'm not sure it's worth designing for.So how would you want this done? Happy to put the work in either way, and happy to be told the shortcut is wrong.
One thing I noticed while testing, in case it's useful when you're deciding: #1324 drops every non-GOAWAY frame once the connection is CLOSED, which stops the exception but the in-flight body goes with it. Ran the mitmproxy repro against it and got a 200 with an empty body. That one targets #1199 though, so it isn't trying to cover this.
@Kriechi sorry to pull you into a seven year old issue nobody's touched since 2022. Mainly after a steer on direction before I write anything up.
varjoranta commentedon Oct 16, 2026
GitHub's API gives a deterministic reproduction of this: api.github.com ends every HTTP/2 connection after 1,000 streams with
GOAWAY(NO_ERROR, last_stream_id=<newest stream> + 200). Sending sequential GETs tohttps://api.github.com/rate_limit(not counted against the rate limit) over one connection, request 1,000 fails even though the server is still answering it. Through httpx/httpcore every in-flight request fails withRemoteProtocolError: <ConnectionTerminated error_code:0, last_stream_id:2199>, and frames for those streams hitInvalid input ... in state ConnectionState.CLOSED.We handled it in a client by following Go's
x/net/http2transport, and it holds up against GitHub and in tests with a loopback server that writes the GOAWAY mid-response:last_stream_idkeep sending and receiving, flow control included; the outbound buffer is kept, since it can hold WINDOW_UPDATEs and SETTINGS ACKs those streams need@neoLsH on your design question: we did this as a subclass that leaves the state machine open and keeps the boundary beside it. That works for one client but has the problem you describe for consumers that check
state is CLOSED, so a distinct draining state looks like the better fit for h2 itself. Happy to review your patch and run it against GitHub's turnover if you open a PR.