max-mapper / websocket-stream

websockets with the node stream API
BSD 2-Clause "Simplified" License
667 stars 114 forks source link

The stream loses data #139

Open DmitryMyadzelets opened 6 years ago

DmitryMyadzelets commented 6 years ago

In this use case we send a file. All data are lost if the file is small, or received partially if the file is big.

How to reproduce:

const fs = require('fs')
const reader = fs.ReadStream('index.js')
const writer = fs.WriteStream('out')
const socket = require('websocket-stream')('ws://echo.websocket.org')

reader.pipe(socket).pipe(writer)

// Now compare the size of the input and output files

Node.js v8.6.0

DmitryMyadzelets commented 6 years ago

Have found that the stream intentionally throws away any data unless the socket is connected. What is a rationale behind such decision?

mcollina commented 6 years ago

This module is very old, and there might not be a good rationale behind some of the code. If you want to help maintaining this you are welcome to propose a change.

DmitryMyadzelets commented 6 years ago

With modern Node.js making a stream over a WebSocket (ws module) seems quite simple for a trivial use case.

May a complete refactoring be considered?

mcollina commented 6 years ago

As long as the API stays similar and the tests passes, I see no problem.