Introspection: disturbed or locked

[annevk]

Fetch has a couple cases where it needs to check whether a stream is disturbed or locked. Is this something that other consumers of Streams need?

For Fetch's Body mixin I plan to introduce a term for this condition: "unusable" (see whatwg/fetch#1155), but it would still be useful to have a term that's applicable to ReadableStream objects as well and ideally that name would be shared.

[ricea]

See also #1025.

[annevk]

Do you think unusable is a reasonable name or would you want something else? It would be nice to avoid having to rename Body's concept later, though it's not the end of the world of course.

[ricea]

Does this mean that response.unusable would be equivalent to response.bodyUsed || response.body.locked?

[annevk]

@ricea it's not a public API and if there was to be a public API it should be on ReadableStream I think.

[ricea]

Oh, sorry, I misunderstood.

We've discourage the use of disturbed and AFAIK no-one outside of Fetch uses it.

[annevk]

Interesting, could Fetch have used something else?

[ricea]

Interesting, could Fetch have used something else?

I wasn't involved in the original discussion, but if I understand it correctly it was a compromise to ensure that response.arrayBuffer() and friends would never return partial output. @yutakahirano probably knows the history better than me.

[annevk]

Looks like yutakahirano/fetch-with-streams#13 has some background and w3c/ServiceWorker#452 (comment) also goes into it. Still not sure I understand why other APIs would not want the same kind of guarantees. Seems @domenic might know as well. (I was involved, but I have forgotten most of that...)

[domenic]

It's hard to read those old threads, as I've lost a lot of the context and it seems like we didn't have the notion of exclusive readers at the time bodyUsed and "disturbed" was added? So maybe the motivation doesn't make sense anymore?

I will say that I continue to believe bodyUsed is not great, and in particular giving information about whether the stream has been read from is not the sort of information programs should generally be branching on. Thus the cautionary note at https://streams.spec.whatwg.org/#is-readable-stream-disturbed

So my inclination for now is that it's best not to add more conveniences around this somewhat-legacy corner of the streams/fetch spec intersection. As @yutakahirano says in #1025, what people are most interested in is validity of a stream, which is only tangentially connected to whether it's been disturbed, and is usually format-specific.

[MattiasBuelens]

I agree with @domenic that we shouldn't add more conveniences around "disturbed".

I think, if I were to design the Body API today, I would have a Body.stream() method (mirroring Blob.stream()) instead of the Body.body property. Calling that method would immediately and unconditionally set Body.bodyUsed to true, so Fetch can manage that flag on its own without needing help from the Streams standard.

Unfortunately, we can't remove Body.body now, so we're stuck with it. But maybe we can recommend future standards which want to expose a ReadableStream, to prefer a stream() method in their API? That way, they can avoid needing to inspect the "disturbed" flag.

[annevk]

That's what File API does (though it's also a bit different as it doesn't have to care about letting things be read once). So the problem is not so much bodyUsed, but rather ReadableStream being accessible without side effects?

But we also use this when extracting, e.g., when constructing a Response from a ReadableStream. Why wouldn't that apply elsewhere?

[MattiasBuelens]

Hmm, good point. I don't see why you shouldn't be allowed to construct a Response from a partially-read or cancelled ReadableStream. 🤔

For example, it's already possible to create a non-disturbed ReadableStream from a disturbed one, thus bypassing this restriction:

let rs1 = new ReadableStream({
  start(c) {
    c.enqueue(new Uint8Array([1, 2, 3]));
    c.enqueue(new Uint8Array([4, 5, 6]));
    c.enqueue(new Uint8Array([7, 8, 9]));
    c.close();
  }
});

// Disturb the stream
let reader1 = rs1.getReader();
await reader1.read(); // { done: false, value: Uint8Array([1, 2, 3]) }
reader1.releaseLock();
new Response(rs1); // throws TypeError: "Response body object should not be disturbed or locked"

// Create a "fresh" stream that continues from rs1
let rs2 = rs1.pipeThrough(new TransformStream());
// or: let [rs2, rs3] = rs1.tee(); rs3.cancel();
let resp = new Response(rs2); // works
await resp.arrayBuffer(); // ArrayBuffer([4, 5, 6, 7, 8, 9])

Once we have ReadableStream.from(), this will become even easier:

new Response(rs1) // throws
new Response(ReadableStream.from(rs1)) // ok

Does anyone have any idea why we have this restriction? 🤷‍♂️

[annevk]

I ran blame and found whatwg/fetch#792. It suggests we could possibly remove it for Response, but that would require some special casing. For Request it seems trickier.

[jasnell]

After having just gone through the implementation for Node.js core, I'd definitely be +1 on a disturbed property on ReadableStream. It wouldn't necessarily stop us from creating new Readers if the stream is not locked but it would help to inform on the current state.