Readable.fromWeb doesn't end on empty string

[renekaesler]

Version

v23.5.0

Platform

Linux xiaomi-mi-pro-gtx 6.8.0-51-generic #52-Ubuntu SMP PREEMPT_DYNAMIC Thu Dec  5 13:09:44 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

import { Readable } from "node:stream";

const response = new Response({
  async *[Symbol.asyncIterator]() {
    yield '' // works only with non-empty string
  }
})

const stream = Readable.fromWeb(response.body);
// const stream = Readable.from('') // works as expected

stream.on('data', chunk => {
  console.log(`data: ${chunk}`)
})

stream.on('end', () => {
  console.log('ended')
});

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior? Why is that the expected behavior?

Similar to the Readable.from stream, the Readable.fromWeb stream should trigger both 'data' & 'end' events.

What do you see instead?

data & end events are only triggered, when the response string is not empty.

Additional information

No response

[jakecastelli]

Readable.from has the same behaviour. If It is a normal async iterator then everything works expected, e.g.

import { Readable } from "node:stream";

async function *yieldEmpty() {
  yield ''
}

const stream = Readable.from(yieldEmpty());

stream.on('data', chunk => {
  console.log(`data: ${chunk.toString()}`)
})

stream.on('end', () => {
  console.log('ended')
});

output:

data: 
ended

Is it possibly something to look into in undici? @nodejs/undici

[KhafraDev]

I believe it's an issue with webstreams

import { Readable } from "node:stream";

const webstream = new ReadableStream({
  start (controller) {
    controller.enqueue(new Uint8Array()) // new Uint8Array([0x20]) works
    controller.close()
  }
})

const stream = Readable.fromWeb(webstream);

stream.on('data', chunk => {
  console.log(`data: ${chunk}`)
})

stream.on('end', () => {
  console.log('ended')
});

[renekaesler]

Thank you for taking the time, @jakecastelli . Sorry, I must have expressed myself a little clumsily...

Yes, Readable.from works with both asynchronous iterators and objects that implement the iterable protocol. I was concerned about the behavior of Readable.fromWeb. The above example probably looks wild, but it's just a heavy reduction of a more complex problem (a virtual file system). However, after further investigations the problem seems to lie in the implementation of Response rather than in Readable.fromWeb.

I am assuming the Response constructor takes an iterable as a body parameter. While not explicitly mentioned, the constructors parameter list makes me to believe that. The following code works in Browser (Chrome, Firefox), but not in node.js:

const iterable = {
  async *[Symbol.asyncIterator]() {
    yield "";
  },
};

const response = new Response(iterable);
const reader = response.body.getReader();

reader.read().finally(() => {
  console.log("done");
});

In node.js the read request is never finishing as soon as the iterator returns an empty string. I think this is actually the cause of the odd Readable.fromWeb behaviour

@KhafraDev : Hehe, you replied quicker than me. Yes... I haven't used a ReadableStream. But shouldn't the reader finish in my case, anyways?

[KhafraDev]

In the first repro you provided it can be replicated without Response, for the newer one there is a bug in undici.

There is likely a bug in Readable.fromWeb as well.

[metcoder95]

It is actually not an issue with the WebStreams, but rather how the Redeable works.

When in byte mode, if the chunk is undefined or the length is equal or less than zero, it does not emit the data event, but rather attempts to read more, skipping the data event.

Seems to be done by design, but cannot really assure that; maybe @ronag has more insights?

[jakecastelli]

But it also skipped the end event as well 🤔

[jakecastelli]

I have put some time to debug this, looks like the code never pass line 150

node/lib/internal/streams/from.js

Lines 150 to 155 in 0e7ec5e

	const { value, done } = iterator.next();
	if (done) {
	readable.push(null);
	return;
	}

from OP's repro.

[metcoder95]

Ha, interesting; when I ran @KhafraDev example as well as the one from the Issue, I get the end event but not the data one; for instance was skipped at the line I shared and jumped immediately the end one. Tho let me double-check once more Today

[metcoder95]

btw, the function that parses the webstream into a Node.js's one comes from adapters:

node/lib/internal/webstreams/adapters.js

Lines 515 to 516 in 542006a

	function newStreamReadableFromReadableStream(readableStream, options = kEmptyObject) {
	if (!isReadableStream(readableStream)) {

or maybe I'm missing something?

[jakecastelli]

Interesting that you can get end event, were you using this code snippet provided by @renekaesler?

import { Readable } from "node:stream";

const response = new Response({
  async *[Symbol.asyncIterator]() {
    yield '' // works only with non-empty string
  }
})

const stream = Readable.fromWeb(response.body);
// const stream = Readable.from('') // works as expected

stream.on('data', chunk => {
  console.log(`data: ${chunk}`)
})

stream.on('end', () => {
  console.log('ended')
});

if so, which node version did you use?

[metcoder95]

Yes, tried in v22 and also the version in main