Merged
Conversation
Motivation: Frames written from a child channel may not be written immediately by the connection channel. For example, a DATA frame written on a stream may be larger than the max frame size imposed on the connection and so the connection channel will have to slice it into multiple frames. Or the connection may not be writable and may delay the transmission of the frame. This behaviour isn't currently observable but is useful to know about. Modifications: - Add a frame delegate which is notified when certain frames are written by the connection channel. Result: Users can observer when headers and data frames are written into the connection channel.
rnro
reviewed
Jul 18, 2025
| /// The maximum number of sequential CONTINUATION frames. | ||
| private let maximumSequentialContinuationFrames: Int | ||
|
|
||
| /// A delegate which is told about frames which have eebn written. |
Contributor
There was a problem hiding this comment.
Suggested change
| /// A delegate which is told about frames which have eebn written. | |
| /// A delegate which is told about frames which have been written. |
rnro
reviewed
Jul 18, 2025
| case .byteBuffer(let buffer): | ||
| delegate.wroteData(buffer, endStream: data.endStream, streamID: frame.streamID) | ||
| case .fileRegion: | ||
| () |
Contributor
There was a problem hiding this comment.
Why do we take no action here? If it's intended should it be documented that we don't notify for file regions?
rnro
reviewed
Jul 18, 2025
| XCTAssertNoThrow(try self.serverChannel.finish()) | ||
| } | ||
|
|
||
| func testFrameDelegateIsCalledForDATAFrames() throws { |
Contributor
There was a problem hiding this comment.
This seems to also assert on header frames, is this name appropriate?
Lukasa
reviewed
Jul 18, 2025
| /// This delegate, when used by the ``NIOHTTP2Handler`` will be called on the event | ||
| /// loop associated with the channel that the handler is a part of. As such you should | ||
| /// avoid doing expensive or blocking work in this delegate. | ||
| public protocol NIOHTTP2FrameDelegate: Sendable { |
Contributor
There was a problem hiding this comment.
Where is the requirement that this is Sendable coming from?
rnro
approved these changes
Jul 18, 2025
Lukasa
approved these changes
Jul 18, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
Frames written from a child channel may not be written immediately by the connection channel. For example, a DATA frame written on a stream may be larger than the max frame size imposed on the connection and so the connection channel will have to slice it into multiple frames. Or the connection may not be writable and may delay the transmission of the frame. This behaviour isn't currently observable but is useful to know about.
Modifications:
Result:
Users can observer when headers and data frames are written into the connection channel.