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

Fix: do not crash when IOStream._read() is called before socket is set #75

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

BWorld
Copy link

@BWorld BWorld commented Mar 18, 2016

I found out that 'sometimes' _read() was called on IOStream before the socket wrapper was able to configure the socket.

Stacktrace:

/var/www/..../node_modules/socket.io-stream/lib/iostream.js:97
this.socket._read(this.id, size);
            ^

TypeError: Cannot read property '_read' of null
   at IOStream._read (/var/www/...../node_modules/socket.io-stream/lib/iostream.js:97:14)
   at IOStream.Readable.read (_stream_readable.js:328:10)
   at resume_ (_stream_readable.js:718:12)
   at nextTickCallbackWith2Args (node.js:455:9)
   at process._tickCallback (node.js:369:17)

I have implemented a test case where the stream is asked to read before the socket was set and now it doesn't crash anymore.

I do not think this is the fix and I am convinced that this problem is caused by something else but I do not know the library well enough to figure that out shortly.

Is it possible that this has todo because of promises being resolved within my listener on the wrapped socket when an upload is done?

My implementation (pseudo code):

var streamSocket = ss(socket);
streamSocket.on('upload', function() {
    var streams = {}, data = null;
    ... some code to populate streams and data based on arguments ...

    ... start loop through collected streams ...
    var writeStream = fs.createWriteStream(somepath);
    collectedStream.pipe(writeStream);
    ... end loop through collected streams ...

});

The inputStream is listening for the 'end' event and resolves the promise created for that stream
Promise.all(streamPromises).then(....); handles some stuff and responds by emitting some event but 'sometimes' it fails by triggering the _read() on the IOStream as mentioned above.

If I missed something or made a mistake please let me know.

@pichouk
Copy link

pichouk commented May 15, 2017

Anything new on this ? I am stuck with the described error...

@jupe
Copy link

jupe commented Jul 5, 2018

@nkzawa please merge this in and create new patch release

@rssk
Copy link

rssk commented Oct 2, 2018

We ran into this issue as well wrapping the listener with a promise, and this fix fixes the issue, it'd definitely be cool to have this put in.

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

Successfully merging this pull request may close these issues.

4 participants