ReadableStream response terminates early...

Describe the bug

Please bear with me as this may well be to spec, but I find it very confusing. I am currently sending a Response that contains a readable stream in its body... it has inconsistent chunk sizes as its just html being streamed.

new Response(readableStream);

The following code is called on the response to pipe it into a node ServerResponse. According to the documentation for node, when .write() returns false it just means that the write could not be flushed all in one go, and some data was queued in user memory. The result of a false here cancels the readable stream as it breaks out of the loop.

whatwg-node/packages/server/src/utils.ts

Lines 287 to 295 in 54a4114

    
           for await (const chunk of asyncIterable) { 
        
             if ( 
        
               !serverResponse 
        
                 // @ts-expect-error http and http2 writes are actually compatible 
        
                 .write(chunk) 
        
             ) { 
        
               break; 
        
             } 
        
           }

To Reproduce

The following code will reproduce the issue, we should not see a log of a cancelled stream here and it should pull 5 times...

import { createServer } from 'node:http';
import { createServerAdapter } from '@whatwg-node/server';

const server = createServer(createServerAdapter((request) => {
  let i = 0, t = 5;
  const stream = new ReadableStream({
    pull(controller) {
      i++;
      console.log('pull ', i);
      if (i === t) {
        controller.close()
      } else {
        controller.enqueue('x'.repeat(20480));
      }
    },
    cancel(reason) {
      console.log('cancel ', reason);
    }
  });
  return new Response(stream, { status: 200 })
}));

server.listen(3001);

Expected behavior

I'm not sure I consider this a completed state, it should in theory wait for a drain before iterating again. It could be done like this...

for await (const chunk of asyncIterable) {
  if (
    !serverResponse
      // @ts-expect-error http and http2 writes are actually compatible
      .write(chunk)
  ) {
    await new Promise(resolve => serverResponse.once('drain', resolve));
  }
}

I'm happy to open a PR, but please let me know if I am doing something stupid here.

Environment:

OS: MacOS
@whatwg-node/server: latest
NodeJS: v20.9.0

We'd love to see a failing PR first :) Thanks for the issue

ok no problem, will tackle this shortly. i have updated the original issue to include more info

@ardatan it appears the test harness does not encounter the same issue, which is very odd. the simple reproduction above does however, you can see an example of the issue running in codesandbox: https://codesandbox.io/p/devbox/frosty-bell-22cckf

whilst adding a test i noticed the following, which seems like a whoopsie:

whatwg-node/packages/server/test/node.spec.ts

Line 11 in 54a4114

it.only('should handle empty responses', async () => {

removing that only, and implementing a test against my change outlined above produces passing tests under the assumption that in the test harness, a readable stream is always cancelled, which doesn't seem to happen in real usage (see codesandbox).

i have opened a PR #1088, but again, it appears i could not get a failing test.

You are using Node's global ReadableStream which we don't recommend.
We recommend "@whatwg-node/fetch" for WHATWG API.

You are using Node's global ReadableStream which we don't recommend. We recommend "@whatwg-node/fetch" for WHATWG API.

bit confused here, I noticed the only place where ReadableStream wasn't imported from @whatwg-node/fetch was the server/src/utils.ts file which was already using it from the global.

edit: unless you mean in the reproduction? which also has the same issue if using the @whatwg-node/fetch implementation. the only difference being that i get an AbortError as the cancel reason.

fyi: We ran into the same issue in combination with @google-cloud/storage and Readable.toWeb:

const bucketFile = bucket.file(path);
const stream = Readable.toWeb(bucketFile.createReadStream());
return new Response(stream, {
  status: 200,
  headers,
});

This code only serves the file partially. We had to apply a patch to @whatwg-node/server to fix it (the suggestion from the initial post).

We ran into this same issue when running graphql-yoga 5.6.2 (with @whatwg-node/server 0.9.44), koa 2.13.4, in bun 1.1.20 - responses were being cut off short.
Setting up the koa, yoga integration like this:
appRouter.all('/graphql', ctx => yoga(ctx.req, ctx.res, ctx))

Making the change in the linked PR fixed the issue!

	for await (const chunk of asyncIterable) {
	if (
	!serverResponse
	// @ts-expect-error http and http2 writes are actually compatible
	.write(chunk)
	) {
	break;
	}
	}