streams: Writable expose `needDrain`

[ronag]

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:

• dst is paused
• src fails 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).

[ronag]

@nodejs/streams

[mcollina]

I would call it writableNeedDrain. Overall, +1.

[ronag]

Just a list of things that needs to be updated in this context:

• pipeline
• Readable.pipe
• OutgoingMessage.writableNeedDrain
• Writable.writableNeedDrain
• Duplex.writableNeedDrain