`SendStream::finish()` no longer errors when connection is closed about quinn

[Ralith]

Thanks for the report! I believe this was intended. In particular, the current behavior is described accurately by the rustdoc:

May fail if [and only if] finish() or reset() was previously called.

The reasoning here was that it's common for peers to routinely stop a stream or close an entire connection upon reading some data without waiting for the stream to be finished. If the sender finishes all streams and considers finish on stopped streams or closed connections to be an error, then this is very likely to produce unhelpful error signals, and may do so unpredictably depending on timing. We've had a number of issues opened due to this behavior in earlier releases.

I appreciate that it's counterintuitive that calling finish succeeds on a connection you know to be closed, but I think this is consistent with the semantics of finish. Consider that a connection might also become closed in the time between calling finish and the next opportunity for the connection to transmit any packets, resulting in the same lack of effect that you get from calling finish after closing.

I'm interested in feedback about how much sense this makes to you, and where we could improve in documentation or API to make things more obvious or convenient. You can reconstruct the previous behavior by waiting on SendStream::stopped after finishing, but I think the errors you might receive from doing so are usually not interesting.

[flub]

I guess it is reasonable to not give an error, after all the peer already knows no more data will be written on the stream if it is closed and as you say stops and finishes may be crossing on the wire anyway.

I think the docs can be a little better though. Right now I'm not sure how to interpret that error and wonder if calling .finish().ok() is the only sensible way to call it. Which means it could not even return anything from an API point of view?

My attempt at rewriting the docs right now would look something like:

Notify the peer that no more data will ever be written to this stream.

After calling this the remote's [RecvStream::read_to_end] will complete. The remote may already have stopped the stream itself, maybe because it knew this situation already from the application-level protocol. This also means the remote is already aware that no more data will be written on this stream so is considered a success for finish().

To wait for the peer to receive all buffered stream data, see stopped().

Mail fail if finishe() or reset() was previously called locally. This error is harmless and serves only to indicate that the caller may have incorrect assumptions about the stream's state.