Repository navigation
stream: Writing null to objectMode Transform is unrecoverable #5711
Description
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
Activity
Fishrock123 commentedon Mar 15, 2016
@nodejs/streams
calvinmetcalf commentedon Mar 15, 2016
maybe have _write do process.nextTick(cb) if chunk === null ?
mcollina commentedon Mar 15, 2016
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 commentedon Mar 15, 2016
@calvinmetcalf Sure, that would work. Is there a reason why you'd discard the write, rather than pass it to
_transform(unlikeundefined,false, etc.)?@mcollina Will do. Calling
.write(null)onWritableinobjectModeworks (it is passed it to_write).mcollina commentedon Mar 15, 2016
@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 commentedon Apr 30, 2017
I think this has been resolved by #6170, if I’m mistaken feel free to re-open.