stream: Writing null to objectMode Transform is unrecoverable

[kevinoid]

If null is written to an instance of stream.Transform, the stream does not complete additional writes, emit additional data, nor emit an 'error' or 'end' event. As an example, consider the following:

var stream = require('stream');
var s = new stream.Transform({
  objectMode: true,
  transform: function(chunk, encoding, callback) {
    console.log('transform', chunk);
    callback(null, chunk);
  }
});
s.on('data', console.log.bind(console, 'data'));
s.on('error', console.log.bind(console, 'error'));
s.on('end', console.log.bind(console, 'end'));

s.write('input1', console.log.bind(console, 'finished write1'));
s.write(null, console.log.bind(console, 'finished write2'));
s.write('input2', console.log.bind(console, 'finished write3'));
s.end();

This prints

transform input1
data input1
finished write1

Instead of write2 either causing an error or being ignored and write3 completing normally followed by 'end'.

This occurs because null writechunk skips ._transform without calling ts.writecb causing Writable to buffer subsequent writes waiting for ._write(null, ...) to complete. This behavior has been present since 26ca7d7.

If there is agreement on the appropriate behavior, I can work up a PR. I would suggest removing the ts.writechunk !== null check so that the value will be passed to ._transform to match the current handling of undefined. If _.transform returns it unchanged, it would be ignored, like undefined. Discarding the write, with or without error, could be reasonable as well.

Thanks for considering,
Kevin

[Fishrock123]

@nodejs/streams

[calvinmetcalf]

maybe have _write do process.nextTick(cb) if chunk === null ?

[mcollina]

I'm not sure what is the proper fix, but give it a go, add at least one unit test covering this issue, and what is your best fix.

Just to clarify, what does it happen if you write null to a Writable?

I agree we need to fix this.

[kevinoid]

@calvinmetcalf Sure, that would work. Is there a reason why you'd discard the write, rather than pass it to _transform (unlike undefined, false, etc.)?

@mcollina Will do. Calling .write(null) on Writable in objectMode works (it is passed it to _write).

[mcollina]

@kevinoid add a unit test also for Writable, just in case we think about removing this feature.

@calvinmetcalf I think the best behavior is to move around all falsey values as they are.

cc @mafintosh :)

[addaleax]

I think this has been resolved by #6170, if I’m mistaken feel free to re-open.