Accept PING frames even when fully quiesced #455
Conversation
Motivation: When the connection is fully quiesced, an endpoint responds with a connection error when it receives a PING frame. This behaviour is unnecessary. Modifications: - Adjust `HTTP2ConnectionStateMachine` to accept PING frames even when the connection is fully quiesced. Result: When the connection is fully quiesced, an endpoint will treat the receipt of a PING frame as it would when the connection is active.
|
Oops. Yes, of course. |
| @available(*, deprecated, message: "Deprecated so deprecated functionality can be tested without warnings") | ||
| func testSuccessfullyReceiveAndSendPingEvenWhenConnectionIsFullyQuiesced() throws { |
There was a problem hiding this comment.
I'm afraid you've fallen into one of the many deprecated-API traps 🪤
This whole test class is deprecated because it's testing deprecated APIs. There's an equivalent set of tests in SimpleClientServerInlineStreamMultiplexerTests where this test should live. I think in this instance your test should just work without modifications.
An aside on the deprecated APIs. The original APIs for multiplexing created child channels for streams where you'd read and write HTTP2Frames.
Each HTTP2Frame holds a stream ID and its payload (headers, data etc.) However, when you open a stream the connection state machine wouldn't know about the stream until you did the first write on that stream.
This means that a client could create stream 1 and then stream 3 but write on stream 3 first. A subsequent write on stream 1 would be a protocol violation (new streams must have higher IDs than previously created streams).
To fix this we added new stream creation APIs where the child channels operate on HTTP2Frame.FramePayloads and only when they do their first write do they get assigned a stream ID.
There was a problem hiding this comment.
Ah, that makes sense. Thanks.
| let goAwayFrame = HTTP2Frame(streamID: .rootStream, payload: .goAway(lastStreamID: .rootStream, errorCode: .noError, opaqueData: nil)) | ||
|
|
||
| // Fully quiesce the connection on the server. | ||
| serverChannel.writeAndFlush(goAwayFrame, promise: nil) |
There was a problem hiding this comment.
The go away frame is strongly coupled with this write so it makes sense to group it with it and the comment.
| let goAwayFrame = HTTP2Frame(streamID: .rootStream, payload: .goAway(lastStreamID: .rootStream, errorCode: .noError, opaqueData: nil)) | |
| // Fully quiesce the connection on the server. | |
| serverChannel.writeAndFlush(goAwayFrame, promise: nil) | |
| // Fully quiesce the connection on the server. | |
| let goAwayFrame = HTTP2Frame(streamID: .rootStream, payload: .goAway(lastStreamID: .rootStream, errorCode: .noError, opaqueData: nil)) | |
| serverChannel.writeAndFlush(goAwayFrame, promise: nil) |
| let pingFrame = HTTP2Frame(streamID: .rootStream, payload: .ping(HTTP2PingData(), ack: false)) | ||
|
|
||
| // Send PING frame to the server. | ||
| clientChannel.writeAndFlush(pingFrame, promise: nil) |
There was a problem hiding this comment.
same here:
| let pingFrame = HTTP2Frame(streamID: .rootStream, payload: .ping(HTTP2PingData(), ack: false)) | |
| // Send PING frame to the server. | |
| clientChannel.writeAndFlush(pingFrame, promise: nil) | |
| // Send PING frame to the server. | |
| let pingFrame = HTTP2Frame(streamID: .rootStream, payload: .ping(HTTP2PingData(), ack: false)) | |
| clientChannel.writeAndFlush(pingFrame, promise: nil) |
|
@swift-server-bot test this please |
Motivation:
When the connection is fully quiesced, an endpoint responds with a connection error when it receives or tries to respond to a PING frame. This behaviour is unnecessary.
Modifications:
HTTP2ConnectionStateMachineto successfully receive and send PING frames even when the connection is fully quiesced.Result:
When the connection is fully quiesced, an endpoint will treat receipt and sending of a PING frame as it would when the connection is active.