Handle write promises correctly #1843
Conversation
| /// A message is ready to be sent. | ||
| case sendMessage(ByteBuffer) | ||
| case sendMessage( | ||
| message: ByteBuffer, |
There was a problem hiding this comment.
at this point it's no longer a message, it could be multiple framed messages (though I realise now that all of the naming here is message based...)
There was a problem hiding this comment.
Hm, you're right. I've renamed these to be more consistent although I did keep awaitMoreMessages and noMoreMessages, since I believe it's not wrong to refer to messages in those situations (as we're either waiting for more messages to arrive, regardless of how we frame them, or we know that no more messages will arrive).
|
|
||
| @testable import GRPCHTTP2Core | ||
|
|
||
| final class GRPCMessageFramerTests: XCTestCase { |
There was a problem hiding this comment.
Can you add some tests which check that promises are cascaded as expected?
There was a problem hiding this comment.
I added a test checking this to the GRPCStreamStateMachine tests, but I also just added (or rather modified an existing one) in the framer tests.
glbrntt
left a comment
glbrntt
left a comment
There was a problem hiding this comment.
Couple of places where we've incorrectly stopped failing a promise but otherwise looks good!
2 participants
We're not currently handling write promises correctly in the server and client stream handlers.
This PR adds proper handling by storing promises linked to each write, and chaining them when they're framed together in a single frame, which can be passed onto the
context/writecall onflush().