delay completion of the send stream until the reset error was delivered

[marten-seemann]

Part of #4428.

The application now needs to somehow retrieve the error generated by receiving the STOP_SENDING frame (or, alternatively, Close or CancelWrite).

[codecov]

Codecov Report

Attention: Patch coverage is 97.22222% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 85.21%. Comparing base (e48e1d4) to head (9dbdfb8).
Report is 12 commits behind head on master.

Files	Patch %	Lines
send_stream.go	97.22%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4445      +/-   ##
==========================================
+ Coverage   84.75%   85.21%   +0.47%     
==========================================
  Files         152      152              
  Lines       14371    14697     +326     
==========================================
+ Hits        12179    12524     +345     
+ Misses       1691     1673      -18     
+ Partials      501      500       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[sukunrt]

I understand this is to avoid sending http datagrams after the stream has received STOP_SENDING.

Can you elaborate on

The application now needs to somehow retrieve the error generated by receiving the STOP_SENDING frame

Is the expectation that users do an empty write on the stream to know if it errors?

Is it better to provide a method to retrieve the state of the stream Or just an IsCompleted method on the stream? This should work on the send side since we want to know whether the stream is open when we are sending the http datagram.

[sukunrt]

How does delaying garbage collection help here?
If the stream has been closed for writing, it's alright if we've removed all references to it from the connection. Writes post garbage collection will error anyway.

[marten-seemann]

How does delaying garbage collection help here? If the stream has been closed for writing, it's alright if we've removed all references to it from the connection. Writes post garbage collection will error anyway.

That's correct. On the QUIC layer, we don't need the map entry anymore. However, the upper layer has no way of discovering that QUIC garbage collected the stream, so it's impossible to keep a map at the application layer (as we need to do in HTTP/3 for datagram support) that doesn't leak (iff the error code is never read), as described in #4428.

Can you elaborate on

The application now needs to somehow retrieve the error generated by receiving the STOP_SENDING frame

Is the expectation that users do an empty write on the stream to know if it errors?

No, that would be an awkward API indeed. For the send stream, the expectation is that you either:

1. repeatedly call Write until it errors, or
2. repeatedly call Write and call Close once you've written everything, or
3. at any point call CancelWrite if you want to abort sending

All the cases listed above lead to garbage collection of the stream.

---

The one thing you must not do is the following: Call Write (once or multiple times) with a nil error every time, and then just walk away.

Before this PR, in this case the stream would have been garbage collected as soon as a STOP_SENDING was received. With this PR, the PR won't be garbage collected (until you call Write again, or Close, or CancelWrite).

---

Does this make sense?

[marten-seemann]

Thanks to @sukunrt for the super helpful review on this PR!