RecvStream::read_exact() API insufficient

[rom1v]

In Quinn, RecvStream::read_exact() returns Result<(), ReadExactError>.

To detect end-of-stream, we can use ReadExactError::FinishedEarly:

FinishedEarly The stream finished before all bytes were read

But then, we are unable to distinguish between end-of-stream at read_exact() boundaries or anywhere else.

For example, if I read packets of fixed-size 512 bytes, an EOS at 0, 512, 1024, 1536 or 2048… is correct (there is no more packets). But an EOS at 153, 781 or 1888 bytes is an error (partial packet), but we could not tell from FinishedEarly alone.

By contrast, in Tokio, AsyncReadExt::read_exact() returns io::Result<usize> (so the usize allows to distinguish both cases).

[djc]

Why do you want to use read_exact() instead of read()?

[rom1v]

Because I want to read a full fixed-size packet at a time (for example).

Of course, I could use read() in a loop to handle partial reads, but this is the purpose of read_exact().

[djc]

If you have discrete application-level packets we would usually recommend using one stream per packet.

[rom1v]

Not if the order of packets is important.

Anyway, my point is about the information provided by read_exact().

For example, one possible fix could be to add a parameter to FinishedEarly:

pub enum ReadExactError {
    FinishedEarly(usize),
    ReadError(ReadError),
}

[djc]

Sure, I'd review that PR.

(Streams also have a well-defined order, so it's still not obvious to me that you can't use streams for this.)

[rom1v]

Sure, I'd review that PR.

So you're ok with FinishedEarly(usize)? Won't that cause compatibility issues (it breaks the API)?

(Streams also have a well-defined order, so it's still not obvious to me that you can't use streams for this.)

(If you send one packet per stream, then the packets may be received in a different order, since streams are independent. Typically, imagine an UDP packet is lost transporting data for stream 7, but no loss occurs on stream 11, the packet sent on stream 11 will be received before that of stream 7. So it might be appropriate to send a sequence of packets on a single stream. It was just an example.)

[djc]

So you're ok with FinishedEarly(usize)? Won't that cause compatibility issues (it breaks the API)?

I think it's a reasonable API, but we won't be able to release it in 0.10.x for compatibility reasons.

(If you send one packet per stream, then the packets may be received in a different order, since streams are independent. Typically, imagine an UDP packet is lost transporting data for stream 7, but no loss occurs on stream 11, the packet sent on stream 11 will be received before that of stream 7. So it might be appropriate to send a sequence of packets on a single stream. It was just an example.)

Sure, but the application can "see" stream order so it could buffer at the application level appropriately, and since it has more context on the data contained in the buffers, it might be able to buffer more effectively. At this point it's just about buffering inside the QUIC protocol stack vs buffering at the application level, the latter is usually better.

[Ralith]

Framing within a stream is also perfectly reasonable, since it leaves you free to read streams out of order for other messages, and buffering in the transport layer can be nice since it gives you flow control for free. +1 to this addition seeming reasonable.

[rom1v]

Sure, I'd review that PR.

1666

[rom1v]

By contrast, in Tokio, AsyncReadExt::read_exact() returns io::Result (so the usize allows to distinguish both cases).

Contrary to what I said, if an EOF occurs when calling AsyncReadExt::read_exact(), an error is reported (doc):

If the operation encounters an “end of file” before completely filling the buffer, it returns an error of the kind ErrorKind::UnexpectedEof. The contents of buf are unspecified in this case.

On success, the usize is always the buffer capacity (so I'm wondering why they don't just return ()).

Anyway, it's still useful to add an usize value to FinishedEarly.