Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

stream: notify user of to pipe state via 'unpipe' event. #3785

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

this does not change the semantics of any of the current Stream methods or events,
but makes only a tiny two line change, which gives the user a lot more power.

firstly, by emitting pipe's internal cleanup function to the dest it gives the option of disconnecting the stream.

secondly, by emitting source.emit('unpipe', dest) it allows the source to know when the pipe is disconnected.

this would allow users to do things like

requestThatMayFail
  .on('pipe', function (source, unpipe) {
    requestThatMayFail.unpipe = unpipe
  })
  .on('error', function (){
    requestThatMayFail.unpipe()
  })

stream
  .pipe(requestThatMayFail)
stream
  .pipe(aVeryReliableStream)

Also, it would become possible to notify the upstream pipeline that the pipe has ended,
which could be necessary for long pipelines.
(such was the intention of: joyent#3759 ) but with out changing any semantics.

Why is the cleanup function passed to the dest's pipe event?

Owner

dominictarr replied Jul 30, 2012

so that the dest can trigger the unpipe if it so wishes.
it's probably gonna be the dest that needs to do so, because say, the dest has errored.
although, with a tee pipe, in the new api, you could maybe pass the particular dest to source.unpipe(dest)

the most useful case for this would be piping some thing to multiple places, for redundancy, expecting some to possibly
fail. hmm, I think this is a sufficiently rare use case to justify a userland stream.

isaacs commented Jul 30, 2012

I'm not opposed to this in principle. However:

  1. It's new API, so can't be in 0.8.
  2. It's going to be deprecated immediately in 0.9, since pipe() is a whole different animal.

Can one of the admins verify this patch?

If you'd like, we can revisit some of these concepts, but closing this as it is stale and would like to explore it with a new pr

@tjfontaine tjfontaine closed this Feb 18, 2014

@joaocgreis joaocgreis pushed a commit to janeasystems/node-v0.x-archive that referenced this pull request Dec 9, 2015

@rvagg rvagg 2015-12-09, Version 5.2.0 (Stable)
Notable changes:

* build:
  - Add support for Intel's VTune JIT profiling when compiled with
    --enable-vtune-profiling. For more information about VTune, see
    https://software.intel.com/en-us/node/544211. (Chunyang Dai) #3785.
  - Properly enable V8 snapshots by default. Due to a configuration
    error, snapshots have been kept off by default when the intention
    is for the feature to be enabled. (Fedor Indutny) #3962.
* crypto:
  - Simplify use of ECDH (Elliptic Curve Diffie-Hellman) objects
    (created via crypto.createECDH(curve_name)) with private keys that
    are not dynamically generated via generateKeys(). The public key
    is now computed when explicitly setting a private key. Added
    validity checks to reduce the possibility of computing weak or
    invalid shared secrets. Also, deprecated the setPublicKey() method
    for ECDH objects as its usage is unnecessary and can lead to
    inconsistent state. (Michael Ruddy) #3511.
  - Update root certificates from the current list stored maintained
    by Mozilla NSS. (Ben Noordhuis) #3951.
  - Multiple CA certificates can now be passed with the ca option to
    TLS methods as an array of strings or in a single new-line
    separated string. (Ben Noordhuis) #4099
* tools: Include a tick processor in core, exposed via the
  --prof-process command-line argument which can be used to process V8
  profiling output files generated when using the --prof command-line
  argument. (Matt Loring) #4021.

PR-URL: nodejs/node#4181
6ca5ea3

@richardlau richardlau pushed a commit to ibmruntimes/node that referenced this pull request Dec 14, 2015

@rvagg rvagg 2015-12-09, Version 5.2.0 (Stable)
Notable changes:

* build:
  - Add support for Intel's VTune JIT profiling when compiled with
    --enable-vtune-profiling. For more information about VTune, see
    https://software.intel.com/en-us/node/544211. (Chunyang Dai) #3785.
  - Properly enable V8 snapshots by default. Due to a configuration
    error, snapshots have been kept off by default when the intention
    is for the feature to be enabled. (Fedor Indutny) #3962.
* crypto:
  - Simplify use of ECDH (Elliptic Curve Diffie-Hellman) objects
    (created via crypto.createECDH(curve_name)) with private keys that
    are not dynamically generated via generateKeys(). The public key
    is now computed when explicitly setting a private key. Added
    validity checks to reduce the possibility of computing weak or
    invalid shared secrets. Also, deprecated the setPublicKey() method
    for ECDH objects as its usage is unnecessary and can lead to
    inconsistent state. (Michael Ruddy) #3511.
  - Update root certificates from the current list stored maintained
    by Mozilla NSS. (Ben Noordhuis) #3951.
  - Multiple CA certificates can now be passed with the ca option to
    TLS methods as an array of strings or in a single new-line
    separated string. (Ben Noordhuis) #4099
* tools: Include a tick processor in core, exposed via the
  --prof-process command-line argument which can be used to process V8
  profiling output files generated when using the --prof command-line
  argument. (Matt Loring) #4021.

PR-URL: nodejs/node#4181
65b86c0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment