Use NodeJS 0.10 streams interface

[josephg]

Browserchannel server should return a nodejs 0.10 stream instead of the fake stream object it currently returns.

[wenzowski]

started in the 2.0 branch at f86796d

[josephg]

There's a problem with this whole change that I didn't realise until recently:

The node v2 stream API will only allow one _write call on the underlying stream to be happening at the same time, and even if you call the _write callback syncronously, _write won't be called again until after the next event loop in a process.nextTick() call.

According to the _write docs:

writable._write(chunk, encoding, callback)

• callback Function Call this function (optionally with an error argument) when you are done processing the supplied chunk.

So, when are we done processing the chunk to send to the client? - Ie, when do we call the callback?

---

Option 1:

We are done processing the chunk when the chunk has been actually sent to the client.

Remember that only one outstanding call will be made to _write. If we do this, the server will never buffer messages, and will only have one message in-flight at a time. Throughput will absolutely plummet.

If _write was called again syncronously, we could pump the stream's write buffer by appending everything its trying to write to an internal buffer. Then in a process.nextTick we could actually send the content to the request. But because of the way _write works, a process.nextTick still wouldn't be able to collect all the messages.

Option 2:

We claim to be done processing the chunk immediately.

The problem with this is that a lot of events that are managed by the stream will fire at the wrong times. As a result, if the user writes code like this:

session.end('last message', function() {
  process.exit();
});

... then the message will never be sent to the client.

---

Primus apparently still uses the old node streams API - maybe that would work. A couple other libraries have been suggested too, including Dominic Tarr's duplex library. But I'm not sure that using a third party library is any better than what we're doing now.

[josephg]

I talked to a lot of people about this, and I'm abandoning the changes for now. There's no reasonable way to implement this on top of the current streams API. I don't think its worth the code churn to move to a different streams library.

I'm assured that node streams 3 is going to address this - which should launch with 0.12.