Repository navigation
streams: non-writable Duplex is writable (more annoyance than bug) #34374
Description
@mcollina @ronag @nodejs/streams
const { Duplex } = require('stream');
const d = new Duplex({
writable: false,
write(chunk, encoding, cb) {
console.log(chunk.toString());
cb();
}
});
console.log(d.writable); // false! as expected...
d.write('darn it'); // prints, 'darn it' and returns trueThis isn't a bug since it's been like this forever but the behavior is really counter intuitive, especially since after calling d.end() and then doing a d.write() we get a proper write after end error. It makes implementing a custom Duplex (e.g. QuicStream) more difficult because of the additional checks that need to be made to ensure that even tho the Duplex isn't writable no-one is writing to it.
Not sure what the fix is immediately but wanted to discuss it first.
The ideal behavior, I would think, is an error similar to write after end if write() is called on a non-writable Duplex.
Activity
jasnell commentedon Jul 15, 2020
Also, just to be thorough, the same issue exists with
readable: falseand the readable side. Specifically:ronag commentedon Jul 15, 2020
What would you like to happen? Error if write/push to a non writable/readable stream?
readable/writable false should be as if end()/push(null) has been called in the constructor but without emitting the associated events?
preyunk commentedon Jul 15, 2020
@ronag @jasnell
Correct me if I am wrong here, according to the readable.readable and writable.writable docs they must be used to check if it is safe to read/write on the stream. That's why setting
writableandreadableexplicitly doesn't seem to work, maybe that's the reason we don't get the option to setwritableandreadableproperty as options while creating a newDuplex.However if I am wrong, the ideal behavior that is throwing an error as proposed by @jasnell looks good.
ronag commentedon Jul 15, 2020
No, they don't need to be checked. Calling it "safe" is maybe a bit misleading, but the following sentence does explain what it means in the context.
It does "work". Depends on what you mean by "works". Though that usage is deprecated.
That option actually does exist and should be used. It's just not documented, which we should fix.
ronag commentedon Jul 15, 2020
@jasnell I'll investigate what we can do to improve this.
preyunk commentedon Jul 15, 2020
By "doesn't work" I meant that we are allowed to read/write on a non readable/writable
DuplexShould I open another issue regarding this? Also are there any more options other than
writableandreadablethat must be included in the docs?ronag commentedon Jul 15, 2020
Sure!
I don't think so.
stream: error Duplex write/read if not writable/readable