Consider having polling an error represent the final Stream value

[carllerche]

Consider having polling an error represent the final value. In other words, a poll that returns an error means that poll should never be called again. In the case of a Stream, this means that an error indicates the stream has terminated.

This issue is a placeholder for the associated discussion.

cc @aturon

[aturon]

@alexcrichton and I debated this initially, and the two options are equally expressive, in that a given stream can be set up to behave either way in either approach (e.g. by using Result in the Item type). So the question is what is the most convenient/straightforward default.

I tend to think that it's much more common for stream errors to be "fatal" than recoverable, so I agree with @carllerche that we've probably chosen the wrong default here.

[alexcrichton]

I also think that this is largely a question of what the combinators do. For example the TCP listener probably won't fuse itself once an error happens but rather will continue to keep accepting sockets. The combinators, however, would be able to assume that when an error happens or Ok(None) happens that everything is terminated.

Sounds reasonable to switch to errors == "end of stream"

Terminating a stream on an error would simplify the life of implementers of the
Stream trait, as they wouldn't have to ensure any particular behaviour of
further polls after returning an error (presuming that behaviour of poll after
termination is still left unspecified).

The main question seems to be indeed, what combinators would do in that case.
They could be overspecified with respect to errors (edit: at least those that
short-circuit on error), and continue to behave exactly the same as they
currently do. In this case the difference would not be particularly important.

I the world with streams terminating on an error, would there still be a place
for combinators that recover from errors like or_else and then?

They could recover from one error, but what next? Their only option seems to be
to terminate the stream, as otherwise they would be breaking the Stream
contract. Notice that it is the combinator that would be breaking the contract
on continued polling, not a downstream consumer.

This seems to be a weak point of stopping on errors. Error recovery is
possible, but only once. Alternatively, those combinators could have stronger
preconditions to work only with streams that permit further polling on errors.

[alexcrichton]

Hm yeah that's a very good point about the combinators. I think this would remove basically all of the "chaining" combinators like and_then, or_else, and then. More specifically:

• or_else would only allow for recovery of one error, which seems odd
• then similarly would only allow catching one error, which seems odd
• and_then would maybe generate errors from futures, not the stream. This error would enter the stream and then oddly terminate it even though the original stream is just fine.

That... may slightly change my opinion here. @carllerche @aturon thoughts about the effect on these chaining combinators with streams?

[carllerche]

IMO this is an argument in favor of having the Stream error terminate the stream.

There are two different error classes with streams:

1. A stream of values that are either OK or errors.
2. An error in the production of values, which means that no further values may be produced due to that error.

The current behavior of stream errors implies the first category which makes the second class of errors difficult to model.

If, however, the stream error terminates the stream, this implies that a stream error follows under group #2.

TcpListener::incoming is group #1, in which case, I believe this is modeled much better with a Stream<Item = Result<T, E>, E2>

This lets you differentiate between producing an error value, and an error in value production (you can have potentially two different error types).

As pointed out, combinators like or_else, and_then, then, etc.. don't make sense anymore, but a different set of combinators do:

incoming
  .map(my_or_else_fn)
  .take_while(Result::is_ok)

For example would provide similar behavior as the current or_else

[LalitMaganti]

I would also favour the approach of termination of stream on errors. I find that an error which terminates the stream is much more common than one which is an "expected value" and is recoverable from.

(FYI Rx also does this and it has error handling "operators" if you want to check how and_then et. all can be modelled in the new system).

[alexcrichton]

Hm I wonder if we'd perhaps have:

fn and_then<F, R>(self, f: F) -> impl Stream<Result<R::Item, R::Error>, Self::Error>
    where F: FnMut(Self::Item) -> R,
          R: Future,

That is, @carllerche I agree that if we had these semantics then a TCP listener would be that form of a stream. I find and_then to be a very useful combinator, however, and it'd be a shame to lose it. If, however, and_then just transformed the Item type then perhaps that could work.

I'm still a little uneasy about the composition story though, it doesn't feel quite right...

[carllerche]

You could do something like this, which would have the additional benefit of being able to have different have different error types for the different category of errors:

fn and_then<F, T, E, U, R>(self, f: F) -> impl Stream<Result<U, E>, Self::Error>
    where Self: Stream<Item = Result<T, E>> + Sized,
          F: FnMut(T) -> R,
          R: IntoFuture<Item = Result<U, E>, Error = Self::Error>
{
    // ...
}

... it is a lot of generics though...

[carllerche]

I think this PR is relevant to the discussion: https://github.com/alexcrichton/futures-rs/pull/199/files

Aka, if an error is passed through without terminating the stream it is unclear if it should count as an element or not.

[alexcrichton]

If we go with this (which it seems like we will) we should update stream::iter to take an iterator of items, not an iterator of results.