Node v15.6.0 changes the behavior of .end on zlib.DeflateRaw

[Aaron1011]

• Version: v15.6.0
• Platform: Linux ArchLaptop 5.10.9-arch1-1 deps: update openssl to 1.0.1j #1 SMP PREEMPT Tue, 19 Jan 2021 22:06:06 +0000 x86_64 GNU/Linux
• Subsystem: zlib

What steps will reproduce the bug?

The following code:

const {DeflateRaw} = require('zlib');

class MyStream extends DeflateRaw {
    constructor(options) {
        super(options);
    }

    write(chunk, enc, cb) {
        console.log("Write: " + chunk + " " + enc + " " + cb);
    }
}

console.log("Running");
const s = new MyStream();
s.end(Buffer.from('hello world'));

produces the following output on Node v15.6.0:

Running

On Node v15.5.0 and below, it produces the output:

Running
Write: hello world undefined undefined

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

This reproduces 100% of the time

What is the expected behavior?

Calling end should call the overridden write method with the supplied data, as in Node v15.5.0 and below.

What do you see instead?

The overridden write method is not called.

Additional information

This leads to node-archiver producing invalid ZIP files: archiverjs/node-archiver#491

PR #36618 seems like it might be related

[Trott]

@nodejs/zlib @nodejs/streams

[lpinca]

The issue seems to be caused by a4fce32. Reverting it makes the example work as expected.

[mcollina]

@ronag wdyt? I have a feeling we should be reverting a4fce32 and add a unit test.

[lpinca]

I think a4fce32 is fine. The documentation does not mention that writable.end() calls writable.write() and writable._write() is still called as expected.

I would only revert on v15.x and mark #36817 as semver-major.

[pgrzesik]

Hello 👋 To clarify - @lpinca - does that mean that the original PR (#36817) that caused issues for some packages depending on undocumented behavior will be merged to the next major version? Thanks in advance 🙇

[mcollina]

Hello 👋 To clarify - @lpinca - does that mean that the original PR (#36817) that caused issues for some packages depending on undocumented behavior will be merged to the next major version? Thanks in advance 🙇

Yes, we do not plan to revert on master, only on v15.

[pgrzesik]

Thanks for clarification @mcollina 🙇

[lpinca]

I'm closing this as it was addressed by #37060.

[severn-everett]

@mcollina Why was this not reverted on master? I'm running into the same issue on 16.3.0.

[ronag]

@severn-everett Because the new behavior is considered correct.