jetstream icon indicating copy to clipboard operation
jetstream copied to clipboard

Consider making all transports derive from a duplex stream

Open robskillington opened this issue 11 years ago • 6 comments

robskillington avatar Nov 02 '14 21:11 robskillington

The advantages of a transport being a duplex stream is that it makes authoring new transports easier ( in fact there are a ton of existing transports that are already duplex streams out of the box ).

The current JetStream implementation expects an instance of AbstractTransport and it expects the Transport to emit SyncProtocolMessage instances.

By making the transport a JSON object stream you decouple it completely from the complexities and concerns of the sync stuff.

Raynos avatar Nov 02 '14 21:11 Raynos

@Raynos the biggest issue I can see with this is that depending on the type of transport you may exchange Pings as at a different interval or on different events.

For example on Websocket transport you exchange Ping to recover missed messages on reconnection, but with a more reliable transport such as over a channel that already does message resending you will likely not do a ping at that time. We need some way to allow specific transports to add this capability while still looking like a Duplex stream. I think we can accomplish this, however I need to evaluate it before committing to it.

It is a very good suggestion and I do like the benefits.

robskillington avatar Nov 02 '14 22:11 robskillington

@robskillington

there is no such thing as a transport that does not have reconnections. If the internet dies then the stream breaks.

There's only one transport i know of that does "automagic" reconnection and its socket.io. That feature is generally a bad idea since your replication algorithm needs to know about offline vs online.

I understand what you mean about transports might behave differently though.

Raynos avatar Nov 02 '14 22:11 Raynos

I'm going to close this, I spent a large time today investigating what this would look like and decided that the interface I'd be implementing are not compatible with the data I'm passing back and forth: http://nodejs.org/docs/latest/api/stream.html#stream_class_stream_writable_1

writable._write(chunk, encoding, callback)#

chunk Buffer | String The chunk to be written. Will always be a buffer unless the decodeStrings option was set to false.
encoding String If the chunk is a string, then this is the encoding type. Ignore chunk is a buffer. Note that chunk will always be a buffer unless the decodeStrings option is explicitly set to false.
callback Function Call this function (optionally with an error argument) when you are done processing the supplied chunk.

For a Transport I'd be writing NetworkMessage objects which means chunk is not an applicable name Buffer or String are not applicable types. There are similar problems with the readable interface.

robskillington avatar Jan 19 '15 04:01 robskillington

@robskillington Streams support objectMode

When running in objectMode the chunk is an arbitrary javascript value. The encoding is ignored.

The same holds true for readable streams, when running in objectMode all the chunks flowing through are treated as arbitrary values (with the exception that null has special meaning, i.e. it means end of stream).

Raynos avatar Jan 19 '15 04:01 Raynos

Alright let me try some more, I want to make sure the interfaces are clean and make sense and I'm not trying to fit a square through a round peg hole.

robskillington avatar Jan 19 '15 04:01 robskillington