Streams that never send data should still close cleanly

[josephnoir]

A stream that never sends remains in the ready state. When closing without and error, this still results in a RESET_STREAM message instead of a clean close (frame with FIN). This PR moves such streams from ready to send when no error was reported and sets closeWrite to false (which is important for packet loss paths?).

[rpaulo]

Sending a RESET_STREAM is perfectly OK. I'm not sure why this is a problem?

[josephnoir]

It fabricates an application error code (in this case 0) that was not generated by the application. RFC Section 11.2 also says:

Resetting a stream without the involvement of the application protocol could cause the application protocol to enter an unrecoverable state. RESET_STREAM MUST only be instigated by the application protocol that uses QUIC.

[rpaulo]

That just means that, if we are closing streams from under the application, it may cause problems.

If the application still closed the stream abruptly, we can still send RESET_STREAM and I would argue that's the correct way to do it because we should not be required to hold on and retransmit any pending data as that was not what the application asked for. It's not clear to me if the code already addresses that.

Also, I think what we need to do is go from ready -> send -> data_sent. Is that happening here? See https://www.rfc-editor.org/rfc/rfc9000.html#name-sending-stream-states

[agnosticdev]

It fabricates an application error code (in this case 0) that was not generated by the application.

Just for my understanding too, this is causing issues at the application layer I am assuming?

[josephnoir]

So far I just ran into this while writing a test. RESET_STREAM surfaces as a thrown error instead of a clean close. It arrives in QUICChannelStreamHandler.handleInboundAbortedEvent(from:error:), where we cannot assume that application error code 0 was not sent by the application.

[josephnoir]

Sorry, I missed this one.

If the application still closed the stream abruptly, we can still send RESET_STREAM and I would argue that's the correct way to do it [...]

Yes, I agree. RESET_STREAM is a valid part of the protocol and should still be supported. In that case the application error code should be set (by the application). The only path I want to change is the close when the application error code is not set (stream.outboundApplicationError == nil) and the stream has not sent any data (it is still in the state ready).

Also, I think what we need to do is go from ready -> send -> data_sent. Is that happening here? See https://www.rfc-editor.org/rfc/rfc9000.html#name-sending-stream-states

This code path is taken when the state is already send (the stream wrote data before the close). I'm expecting that this will follow the same state transitions from here on. It should not be in data_sent yet because it still needs to send a stream frame with a FIN.

I'll check if I can improve the test case to be more explicit about this.

[agnosticdev]

So far I just ran into this while writing a test. RESET_STREAM surfaces as a thrown error instead of a clean close. It arrives in QUICChannelStreamHandler.handleInboundAbortedEvent(from:error:), where we cannot assume that application error code 0 was not sent by the application.

Yeah, that makes sense to address, thank you @josephnoir