only write on .pipe #23

Closed
juliangruber opened this Issue Mar 6, 2013 · 4 comments

3 participants

@juliangruber

Me and @ralphtheninja had this problem that MuxDemux wrote its meta infos before mux-demux was piped anywhere, like:

// Server
var mdm = require('mux-demux')()
net.createServer(function (c) {
  c.pipe(mdm).pipe(c)
}).listen(3000)

// Client
var mdm = require('mux-demux')()
mdm.pipe(net.connect(3000)).pipe(mdm)

If you inspect the traffic you see that the server-mdm wrote its meta before the connection to the client was up. What I saw you doing to avoid this is you create the mdm object inside the net.createServer-handler.

That works for this case, but libraries using mux-demux (like multilevel) have the problem that they can't expose a simple stream interface anymore.

// Breaks
var server = multilevel.server('/db')
net.createServer(function (c) {
  c.pipe(server).pipe(c)
})

// Works
net.createServer(function (c) {
  c.pipe(multilevel.server('/db')).pipe(c)
})

multilevel directly exports its underlying MuxDemux-instance. What I could do is wrap that in a paused duplex-stream and listen for mdm.on('pipe', fn) but I think that this should happen inside mux-demux, in order not to leak its implementation details.

If you want I can hack up a PR that for every pipe event emits its stream meta.

@dominictarr
Owner

You should create a mux-demux immediately before you pipe it, or explicitly pause it.

This will work:

var server = multilevel.server('/db').pause()
net.createServer(function (c) {
  c.pipe(server.resume()).pipe(c)
})
@ralphtheninja

Just tried it in https://github.com/juliangruber/multilevel/blob/master/test/util.js

This doesn't work:

    var server = multilevel.server(__dirname + '/db').pause()
    net.createServer(function (con) {
      con.pipe(server.resume()).pipe(con)
    }).listen(port)
@dominictarr
Owner

oh, oops

just realized, resume() resumes immediately, so you need to

var server = multilevel.server(__dirname + '/db').pause()
    net.createServer(function (con) {
      con.pipe(server).pipe(con)
      server.resume()
    }).listen(port)

sorry.

returning it prepaused is unusual. If really want to do that make sure you document it CLEARLY.

Generally, you shouldn't need to create a stream before you pipe it, so this is best:

net.createServer(function (con) {
   con.pipe(multilevel.server(__dirname + '/db')).pipe(con)
}).listen(port)

actually, this is wrong:

var server = multilevel.server(__dirname + '/db').pause()
    net.createServer(function (con) {
      con.pipe(server).pipe(con)
      server.resume()
    }).listen(port)

because when multiple connections join, they will all be piped onto the same stream.
instead, you want a stream from multilevel for each connection.

Also, can you alias multilevel.server and multilevel.client to multilevel.createServerStream and multilevel.createClientStream as it will make their behaviour more obvious?

@juliangruber

ok. @substack I think that "you shouldn't need to create a stream before you pipe it" pattern should go in the stream handbook?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment