nodejs / readable-stream

Node-core streams for userland
https://nodejs.org/api/stream.html
Other
1.03k stars 227 forks source link

May 2017 wg meeting: in-person @ Collaborator Summit #275

Closed mcollina closed 7 years ago

mcollina commented 7 years ago

I think we need to sketch out an agenda for what we want to do here, as there are several things to be talked about. I'm opening this as a placeholder:

Given we have time during the wg, we might even decide to pair up and do things.

ref: https://github.com/nodejs/summit/issues/41.

I know @calvinmetcalf @yoshuawuyts @lrlna @addaleax and myself (@mcollina) will be there.

@mafintosh, would you be able to join?

yunong commented 7 years ago

Can we also talk about documentation of the streams API? I'd like to have the experts in this group discuss the streams API and implementation, take notes, and then distill that into docs that's easily accessible by users of Node.

addaleax commented 7 years ago

@yunong yes! I don’t think you could get a better opportunity. :)

calvinmetcalf commented 7 years ago

other idea: duck typeable api, like what are the most basic methods that say a readable and writable streams need to implement so that if we want to create streamish objects.

calvinmetcalf commented 7 years ago

idea: ability to turn off the complexity of streams2, aka neo stream 1 object type thing

calvinmetcalf commented 7 years ago

official pump method

edit: or pipeTo/pipeThrough methods

mcollina commented 7 years ago

Meet with the build wg to help moving the testing infrastructure @nodejs/build #270

Agreed for tomorrow agenda.

yosuke-furukawa commented 7 years ago

This is the very old article, but some stream beginner drops this trap. http://grokbase.com/t/gg/nodejs/12bwd4zm4x/should-stream-pipe-forward-errors

rstream
   .pipe(foo)
   .pipe(bar)
   .on('error', function (err) {
     // handle the error
   });

but stream should forward some errors to pipe.

rstream
   .on('error', handleError)
   .pipe(foo)
   .on('error', handleError)
   .pipe(bar)
   .on('error', handleError);

officially, izs answered about that,

For example, let's say that you added something like this in the
pipe() function:

     src.on('error', function(er) {
       dest.emit('error', er);
     });

What happens when you do this?

     // encryptor service
     net.createServer(function (socket) {
       socket.pipe(new Encryptor()).pipe(socket);
     });

If the socket emits error, it forwards to the encryptor, which
forwards it to the socket, and it's an infinite loop. Now every
encryption error is also a RangeError!

-- snip --

if you really *wanted*
to only have a single error handler you could do this:

     var d = domain.create();
     d.on('error', handleAllErrors);
     d.run(function() {
         fs.createReadStream(tarball)
           .pipe(gzip.Gunzip())
           .pipe(tar.Extract({ path: targetPath }))
           .on('close', cb);
     });

Domains add a bit of metadata to the
error object, so it's not too hard to figure out whether it was an
"organic" throw, or an error event that was emitted by some object.

but currently domains are deprecated. do you have a plan to add stream APIs to propagate errors ???

mcollina commented 7 years ago

Standardize destroy() #125

Method signatures: _destroy(error, cb) and destroy().

mcollina commented 7 years ago

Support UInt8Array throughout the API nodejs/node#11608

If it works in browsers/older nodes is good, LGTM cc @addaleax as semver-minor

mcollina commented 7 years ago

error codes for streams cc @jasnell

we will setup an issue to describe the migration logic for readable-stream. We cannot use custom classes, so we will need to do:

var err = new Error('this is a long message');
err.name = 'ThisIsMyError';
err.code = 'ERROR_CODE';
throw err;

The error code will need to be lifted from: https://github.com/nodejs/node/blob/master/lib/internal/errors.js.