Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

When should closed promise be resolved? #1244

Closed
davedoesdev opened this issue Oct 16, 2022 · 2 comments
Closed

When should closed promise be resolved? #1244

davedoesdev opened this issue Oct 16, 2022 · 2 comments

Comments

@davedoesdev
Copy link

I've got a question about stream reader closed promise, specifically whether it should resolve before all enqueued data has been read.

In the program below, I'm seeing the message data not read yet being logged. I was assuming reader.closed wouldn't get resolved until the program had read the data.

const rs = new ReadableStream({
  start(controller) {
    controller.enqueue(new Uint8Array(3));
    controller.close();
  }
});

let data = null;
const reader = rs.getReader();

reader.closed.then(() => {
  if (!data) {
    console.error('data not read yet');
  }
});

data = await reader.read();
console.log(data);

data = await reader.read();
console.log(data);

I'm seeing this in Chrome, Firefox and Node.

So I read the spec and in https://streams.spec.whatwg.org/#rs-default-controller-private-pull ReadableStreamClose is performed before the read request's chunk steps which explains what I'm seeing. (Indeed I looked at Node's implementation and it follows the spec).

Is this how closed is supposed to behave? It has implications for the calling application. For example, Node uses it to push EOF in its stream adapter and this causes nodejs/node#42694

@domenic
Copy link
Member

domenic commented Oct 16, 2022

So, the promise isn't resolved until the program has read the data. That is true. The issue is just that, once the data is read, closed is resolved... and the read() promise is resolved... but you added your handler to the closed promise before you added your handler to the read() promise, so the closed handler gets called first.

You can see this a bit more clearly with https://jsbin.com/quxonoxoye/1/edit?html,js,console . If you reorder the three .then() lines, you get the results printed in whatever order you added the handler.

This is just how JS promises work. For any promises which are resolved in the same turn, it doesn't matter what order they are resolved in; it only matters what order they are subscribed to in. An even simpler example to show this is https://jsbin.com/jiviniveli/1/edit?html,js,console .

Thus, I'm pretty sure that even if we moved ReadableStreamClose after the chunk steps, then the code you give would still print "data not read yet", because you subscribed to the closed promise before you subscribed to the read() promise.

I don't think there's much for the spec to do here...

@davedoesdev
Copy link
Author

Thanks for explanation. I'll close this issue.
Node is subscribing to the closed promise first which explains its issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants