Repository navigation
streams: Writable expose needDrain #35341
Description
I have a case where it would make sense to expose needDrain on the public API.
This is when writing to a dst with retries:
Consider:
function send(src, dst, callback) {
src.on('data', (buf) => {
if (!dst.write(buf)) {
src.pause()
}
}).on('end', () => {
src.end()
callback()
}).on('error', (err) => {
callback(err)
})
}Now consider the case where:
dstis pausedsrcfails due to a timeout (e.g. because dst is to slow to read)- the outer caller retries
Since we don't know whether to continue to write to dst until after we call dst.write() every retry will write more data that exceeds the HWM and eventually crashes the process in worst case.
For correctness IMO the above should look like this:
function send(src, dst, callback) {
src.pause()
if (src.needDrain) {
src
.on('error', err => callback(err))
.on('drain', () => send(src, dst, callback)
return
}
src.on('data', (buf) => {
if (!dst.write(buf)) {
src.pause()
}
}).on('end', () => {
src.end()
callback()
}).on('error', (err) => {
callback(err)
}).resume()
}However, we currently don't have a needDrain property in the public API (and especially not OutgoingMessage where I encountered this problem).
Activity
ronag commentedon Sep 25, 2020
@nodejs/streams
mcollina commentedon Sep 25, 2020
I would call it
writableNeedDrain. Overall, +1.ronag commentedon Sep 25, 2020
Just a list of things that needs to be updated in this context:
pipelineReadable.pipeOutgoingMessage.writableNeedDrainWritable.writableNeedDrainDuplex.writableNeedDrainfix: Client.stream writableNeedDrain
fix: Client.stream writableNeedDrain
fix: Client.stream writableNeedDrain
stream: writableNeedDrain
fix: Client.stream writableNeedDrain
fix: Client.stream writableNeedDrain
fix: Client.stream writableNeedDrain
stream: writableNeedDrain
stream: writableNeedDrain
stream: writableNeedDrain