piping to a stream with highWaterMark: 0 and objectMode: false is broken since node v20.10.0

[dy-dx]

Version

v21.6.2

Platform

Darwin x86_64

Subsystem

stream

What steps will reproduce the bug?

const { Readable, Writable } = require('stream');

const r = new Readable({
  read() {}
});

const w = new Writable({
  highWaterMark: 0,
  write(chunk, encoding, callback) {
    console.log(chunk.toString());
    callback();
  },
});

r.pipe(w);

r.push('hello');
r.push('world');
r.push(null);

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

Always

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

In node v20.9.0 and earlier:

hello
world

What do you see instead?

In node v20.10.0 and later, only the first chunk ever arrives at the destination:

hello

Additional information

The issue is present on node v20.10.0, but is not present on v20.9.0. The issue is also present on v21.0.0.

A workaround is to use highWaterMark: 1.

[dy-dx]

After building nodejs locally, I can confirm this change caused the issue: #50014

[IlyasShabi]

I will investigate this later today

[orgads]

@IlyasShabi any progress with this? We hit a similar issue with a custom Writable that calls its callback with an Error.

[ronag]

Here is a possible fix if someone wants to take a look at this:

diff --git a/lib/internal/streams/writable.js b/lib/internal/streams/writable.js
index 0dbf56d7a69..848247c96e5 100644
--- a/lib/internal/streams/writable.js
+++ b/lib/internal/streams/writable.js
@@ -565,7 +565,7 @@ function writeOrBuffer(stream, state, chunk, encoding, callback) {
     state[kState] &= ~kSync;
   }
 
-  const ret = state.length < state.highWaterMark;
+  const ret = state.length === 0 || state.length < state.highWaterMark;
 
   if (!ret) {
     state[kState] |= kNeedDrain;

[ronag]

In general I would say that a highWaterMark of 0 doesn't make much sense...

[jakecastelli]

I will take a look at this 👀

[jakecastelli]

In general I would say that a highWaterMark of 0 doesn't make much sense...

I have linked the spec related discussion here just for visibility and potentially future discussion, thanks @vweevers