Warning misbehaving underlying sources

[tyoshino]

(Branched from #296 (comment))

Should we throw if an underlying source looks misbehaving?

Example: Underlying source calling close function provided by the stream twice or more

[domenic]

I think we should. Here are the cases I can think of:

• close() or error() after stream is already draining or closed or errored (currently ignored)
• enqueue() after stream is already draining or closed or errored (currently throws)

I will do a quick PR to fix this. It seems likely just an oversight so far.

[domenic]

Oh, hmm, what if the stream was cancelled by rs.getReader().cancel()? E.g. I think https://streams.spec.whatwg.org/#example-rs-push-no-backpressure might trigger a call to close() after the stream was already closed.

[tyoshino]

OK. So, shall we warn the source only if close()-ed when it's already draining?

[tyoshino]

Re: error,

It seems we don't have any path to notify the underlying source of that the [[error]] has been called (until the source calls enqueue()), and that's because the only sources of error which results in invoking [[error]] are the strategy or the underlying source itself? So, ...:

1. the source cannot know when an error occurred
2. there's no way for the consumer to error the stream

Hmm, though (1) means that the source cannot avoid bad error()/close() call, given (2), I think it's fine to throw for error()/close() from a source on an errored stream.

---

Re: close,

Hmm, maybe that's the only place where unsolicited close() happens? So, ok, the consumer is no longer interested in data, and the underlying source has finished work. Sounds like it's ok to just make close() after reader.cancel() no-op (no throwing) for convenience though the underlying source CAN know when cancel() happened.

I.e. throw only when draining.

[domenic]

I think your analysis makes sense. Should we allow double-close when not draining, even if both of them were underlying-source initiated? Or should we try to disallow double-close() in that case?

(The only way to make that work I think is to add another flag, or maybe another state which we treat very much like closed. So that's kind of annoying. But it seems also weird that if the queue has chunks in it, close(); close(); will throw, whereas if all chunks have been read, close(); close(); would work fine.)

[tyoshino]

Ah, I see. CreateReadableStreamCloseFunction() doesn't set [[draining]] when the queue is already empty. I guess we can just set draining to true even when in "waiting" state. It was unnecessary but seems doing so is harmless.

[domenic]

Oh I see, that should work pretty well, nice :)