Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

streams: Writable expose needDrain #35341

Copy link
Copy link
@ronag

Description

@ronag
Issue body actions

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).

Metadata

Metadata

Assignees

Labels

streamIssues and PRs related to the stream subsystem.Issues and PRs related to the stream subsystem.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions

    Morty Proxy This is a proxified and sanitized view of the page, visit original site.