Semantics of stream read yielding None and Some(Err)?

[wbrickner]

I am confused on some of the semantics surrounding streams
yielding Option<Result<...>> in tokio-tungsteninte.

Let's say I have a setup like this:

let (stream, _) = connect_async( /* ... */ ).await;
let (_, mut rx) = socket.split();

loop {
  match read.next().await {
    Some(Ok(msg)) => { /* ... */ },

    // The stream has not ended, but there was an error with reading from the socket,
    // but an error not sufficient to close the stream? What?
    Some(Err(e)) => { },

    // The stream has ended... so this is a distinct type of error with the socket? What?
    None => {  }
  }
}

I have witnessed streams yield Some(Err(Protocol(...))) and None (I think separately).

• What are the conditions in which read.next().await yields None? I want to know what this means must have happened.
• Why can a stream yield an error (e.g. a Protocol error – seemingly fatal) and not be ended (by a None)? Is this just an artifact of the stream abstraction / type design?

Thanks so much,
I really enjoy using your library in my daily work, I hope you can help me with my question.

[daniel-abramov]

So the thing is that every stream must eventually end. Like every iterator, when the iterator ends, it returns None, you can view streams as an abstract/future-aware version of a normal iterator from the standard library. So the stream retuning Option is a generic requirement for all streams that we had to implement which also allows all that "interop" with other StreamExt functions, allowing for_each, fold, forward etc.

This also means that if the connection is ended gracefully on either of the sides, we don't really want to return any error, but "gracefully" close the stream (by returning None). In tungstenite-rs (core) the function that deals with reading the messages returns Result<X, Error>, where Error::ConnectionClosed means a normal close of the connection. What we do on tokio-tungstenite is we turn Error::ConnectionClosed into None on the stream, signifying a good graceful closure of the connection.

However sometimes there are cases when the connection is closed because of some error like protocol violation. We don't really want to just turn these into None (as it would hide the abnormal closure of the connection from the end user without giving any information on why the connection went closed). Because of this, the return type of the stream in our case is Option<Result<X, Error>>. This means that if you get Some(Err(...)), then you know that some error occurred and the stream is pretty much not usable anymore, but you know that it was abnormal closure (and can act upon that or ignore it if it's not relevant and just treat it as a general [though abnormal] connection close). Does it clarify things a bit?

[wbrickner]

Yes that clears everything up neatly, thank you.

None is a connection close (in my case initiated by the remote),

Some(Err(m)) is a proper error that has rendered the stream unreadable.

Is this in the documentation? I suspect other people will also not intuit the subtlety, and I think it's important for consumers to understand exactly what conditions they are really handling.

[daniel-abramov]

Is this in the documentation? I suspect other people will also not intuit the subtlety, and I think it's important for consumers to understand exactly what conditions they are really handling.

So the Option is part of the Stream declaration. It's basically something that each entity that implements Stream should care about. In our case we thought it would be logical to treat the stream as exhausted once it's closed.

As for the internal Result of reading, we documented it in tungstenite-rs. But for tokio-tungstenite users it is simpler since in most cases they probably won't handle errors in any special way if they just use the stream with other combinators (to forward it or for_each over the messages). Though it's of course up to the user to define the logic.

[elbaro]

If Some(Err(...)) is unrecoverable, isn't it more natural for WebSocketStream to always return None after Some(Err(...))?

#204 (comment)

[daniel-abramov]

Ok, it makes sense, submitted a PR to address it.