ReadableStream internal state

While working implementing fetch in undici I've noticed a limitation on how compliant we can be towards the spec.

In order to "extract" a request body we need to have access to the streams [[disturbed]] state. However, this state is inaccessible through the public API.

Is there a way we could make this accessible behind a public symbol or something? Otherwise fetch can't be fully implemented outside of core as per my understanding of the specification.

@jasnell

@jasnell @benjamingr

@nodejs/whatwg-stream

@domenic

I'm +1 to expose this via a Symbol('nodejs.webstreams.disturbed').

@domenic wdyt?

I'll reiterate:

We can't add properties or symbols to the stream, that would violate the spec. That point is very clear since it would hurt interop. Just like IE adding random properties to NodeList back when
We are welcome to add statics or anything that is part of core and has access to internal state without changing the standard interface.

This is why we have a static getEventListeners rather than modifying EventTarget for example.

I vote a static disturbed (or better an object that exposes all of these) that works on both web and Node.js streams

We can't add properties or symbols to the stream, that would violate the spec.

We'll end up adding something anyway as we would not have any other way to accessing this state. We might just decide to not expose this Symbol and have it in a not-enumberable property... however we'll need to have it there anyway.

We can't add properties or symbols to the stream, that would violate the spec.

We'll end up adding something anyway as we would not have any other way to accessing this state. We might just decide to not expose this Symbol and have it in a not-enumberable property... however we'll need to have it there anyway.

We already add symbols?

however we'll need to have it there anyway.

Adding things we don't consider our public API is risky but fine as long as it's implementation detail and we don't rely on it. If we can't even build without the internal API - that's a good indication we should expose that API (via a static method).

Since there is a simple, spec-compliant workaround here (a static method) I suggest we stick to that :)

(If we need an API like isDisturbed - contributing back to whatwg is also a good viable option, I am super busy with baby/new-job/new-house/family-health-stuff but technically that can be something I work on in my job, just not in the coming months)

whatwg/streams#1025 this looks positive

Adding non-spec statics is not spec-compliant; I'm not sure why you'd think they're any different from prototype properties.

I would add separate methods that are not hung off the classes at all.

const { isDisturbed, isFinished } = require ('stream')

isDisturbed(readable)

isFinished(readable)

The challenge with these is that they will have to rely on internal state and not public API but that's fine.

stream: add isDisturbed helper

@domenic

@domenic I thought adding statics that are not part of the spec does not violate the spec while adding methods to the interface does. Did I misunderstand?

const s = getStream();
s.isDisturbed(); // not allowed, adds a method to the prototype
ReadableStream.isDisturbed(s); // not allowed, adds a method to ReadableStream
const { isDisturbed } = await import('nodejs/stream-utils');
isDisturbed(s); // this is fine

@ronag

@ronag

My understanding is - fetch is going to be part of Node, web streams are native to Node. This sort of thing can be a problem if we want to make it possible to implement fetch on top of web streams in user-land. Do I understand incorrectly?

Correct. We need it to implement fetch in user land outside of core.

If async suffices the following should work I believe:

async function isReadable(reader) {
  let readable = true;
  reader.closed.then(() => { readable = false; }, () => { readable = false; });
  await Promise.resolve();

  // If the stream was closed or errored then the closed promise would have settled by now and updated readable.
  // If not then it's readable.
  return readable;
}

This is untested. In particular you might need more than one await Promise.resolve() in case the process of signaling the closed promise goes through a number of promise chains. await new Promise(setTimeout) should definitely work.

@ronag

@ronag:

I don't think you are going to be happy with me... but I found a way

You're right! Don't do that lol.

@domenic ... one challenge we're going to have here is that we need to be able to find out the readable/locked/closed/errored state when we likely won't have access to any attached reader.

I've been thinking about opening a spec proposal to add a couple of new getters to the ReadableStream and WritableStream classes. Specifically, stream.locked and stream.status, both of which would be read-only. Before I do so, however, it would be great to get your opinion on it.

Stream.locked already exists, no?

Well, only the person with the reader should be able to introspect the stream; that's a design goal.

stream: add isDisturbed helper

@domenic

What's about @domenic's example:

async function isReadable(reader) {
  let readable = true;
  reader.closed.then(() => { readable = false; }, () => { readable = false; });
  await Promise.resolve();

  // If the stream was closed or errored then the closed promise would have settled by now and updated readable.
  // If not then it's readable.
  return readable;
}

I'll copy my opinion over it from whatwg/streams#1025:

It creates 3 promises and 3 jumps over event loop stack just to determine stream state. Moreover function scope with its boolean, function and one of this promises will live as long as stream is not closed and thus would hold memory. In the case of high loads, this will be memory pressure out of nowhere. This check should be made in a single synchronous call.

So having synchronous check in the case of directly owned a) non-nested undisturbed stream and b) disturbed or closed streams (I expect it as the most cases) all of extra payload will be eliminated, and only indirectly owned undisturbed or closing nested streams would require read attempt to fail. Thus provided code is doing redundant work.

This issue has been marked as stale due to 210 days of inactivity.
It will be automatically closed in 30 days if no further activity occurs. If this is still relevant, please leave a comment or update it to keep it open.

This issue has been automatically closed after 30 days of inactivity following its stale status (no activity for a total of 120 days).
If this is still relevant, feel free to reopen it or leave a comment with additional details so we can continue the discussion.

Navigation Menu

Repository navigation

ReadableStream internal state #39627

7 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

ReadableStream internal state #39627

Description

Activity

ronag commented on Aug 2, 2021

ronag commented on Aug 2, 2021

mcollina commented on Aug 2, 2021

benjamingr commented on Aug 2, 2021

mcollina commented on Aug 2, 2021

ronag commented on Aug 2, 2021

benjamingr commented on Aug 2, 2021

benjamingr commented on Aug 2, 2021

domenic commented on Aug 2, 2021

jasnell commented on Aug 2, 2021

benjamingr commented on Aug 3, 2021

7 remaining items

benjamingr commented on Aug 11, 2021

ronag commented on Aug 11, 2021

domenic commented on Aug 11, 2021

jasnell commented on Aug 11, 2021

ronag commented on Aug 11, 2021

domenic commented on Aug 11, 2021

rumkin commented on Oct 18, 2021

github-actions commented on Jun 27, 2026

github-actions commented on Jul 28, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions