Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

encode stream does not proactively send data to its output #80

Open
josephg opened this issue Jan 10, 2018 · 5 comments
Open

encode stream does not proactively send data to its output #80

josephg opened this issue Jan 10, 2018 · 5 comments

Comments

@josephg
Copy link

josephg commented Jan 10, 2018

If you create an encode stream and pipe it somewhere (for example, the network), the data you send to the stream won't be forwarded automatically. It only seems to be sent when the server end()'s the writable stream. This makes the transform stream basically unusable for streaming realtime data.

There's a workaround of calling stream.encoder.flush(), but that seems brittle.

@josephg
Copy link
Author

josephg commented Jan 10, 2018

Example:

const msgpack = require("msgpack-lite")
const net = require('net')

net.createServer(socket => {
  const encodeStream = msgpack.createEncodeStream()
  encodeStream.pipe(socket)

  // Send 'hi' to the socket every second
  setInterval(() => encodeStream.write('hi'), 1000)
}).listen(3300)

console.log('TCP server listening on port 3300')

This server should send a msgpack message containing 'hi' to each client every second. Instead the stream internally buffers the small messages and nothing gets sent to the network socket.

$ nc localhost 3300
   # No output!?

The problem is avoided by calling this.encoder.flush(); inside the _transform method of encode-stream.js. It seems sort of inelegant though.

@rjeczalik
Copy link

I bumped into this issue as well, as a workaround I'm calling _flush() each write on the encode-stream.

Would extending createEncodeStream with optional options argument make sense? The API would look like:

net.createServer(socket => {
  const encodeStream = msgpack.createEncodeStream({autoflush: true})
  encodeStream.pipe(socket)

  // Send 'hi' to the socket every second
  setInterval(() => encodeStream.write('hi'), 1000)
}).listen(3300)

This would make write() flush the data each call to it, without user code needing to call methods it shouldn't be calling.

@josephg
Copy link
Author

josephg commented Mar 1, 2018

Yeah that'd do the trick!

@kawanet
Copy link
Owner

kawanet commented Mar 1, 2018

Right.

msgpack.createEncodeStream() instance does not manage Stream buffer for each item.
One item might be splitted for multiple chunks encoded or buffered for while.
Multiple items might be joined for a single chunk.

Using it with fs Stream would work great because memory copy operations reduced.
Using it with net Stream may not cause trouble in some cases when chunks splitted.

I rather use the simple msgpack.encode() interface for WebSocket messaging applications.

@rjeczalik
Copy link

@kawanet WebSocket is message-oriented, so msgpack.encode() will work. How to handle streaming msgpack e.g. over raw tcp socket? Like this simple echo server:

const echo = (value, enc) => {
  enc.write(value)
  enc._flush() // required, otherwise client won't get the the response
}

net.createServer(socket => {
  const enc = msgpack.createEncodeStream()
  const dec = msgpack.createDecodeStream()

  enc.pipe(socket)
  socket.pipe(dec)

  dec.on('data', value => echo(value, enc))
}).listen(3300)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants