Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix for crit bug when closing pipe stream #74

Open
wants to merge 1 commit into from

2 participants

@psychowico

When we use pipe method of BinaryStream and close connected browser page - server got crit error:

TypeError: Cannot call method '_onClose' of undefined
  at WebSocket.<anonymous> (/srv/http/socialstream/server/node_modules/binaryjs/lib/client.js:59:28)
  at WebSocket.EventEmitter.emit (events.js:117:20)
  at WebSocket.cleanupWebsocketResources (/srv/http/socialstream/server/node_modules/binaryjs/node_modules/streamws/lib/WebSocket.js:619:23)
  at Socket.EventEmitter.emit (events.js:117:20)
  at _stream_readable.js:920:16
  at process._tickCallback (node.js:415:13)

It is fast fix for this. It easy to found reason by following callstack - when stream is closed it automatically close his pipe destination stream and removed it from self.streams array.

@dominicklim dominicklim commented on the diff
lib/client.js
@@ -54,7 +54,9 @@ function BinaryClient(socket, options) {
this._socket.addEventListener('close', function(code, message){
var ids = Object.keys(self.streams);
for (var i = 0, ii = ids.length; i < ii; i++) {
- self.streams[ids[i]]._onClose();
+ if(!!self.streams[ids[i]]) {
@dominicklim Collaborator

I don't think we need this check since we're just iterating over stream ids.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dominicklim dominicklim commented on the diff
lib/client.js
@@ -54,7 +54,9 @@ function BinaryClient(socket, options) {
this._socket.addEventListener('close', function(code, message){
var ids = Object.keys(self.streams);
for (var i = 0, ii = ids.length; i < ii; i++) {
- self.streams[ids[i]]._onClose();
+ if(!!self.streams[ids[i]]) {
+ self.streams[ids[i]]._onClose();
+ }
}
self.emit('close', code, message);
@dominicklim Collaborator

It seems like the actual problem is here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dominicklim
Collaborator

Do you have any steps to reproduce this error?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 24, 2014
  1. @psychowico
This page is out of date. Refresh to see the latest.
Showing with 3 additions and 1 deletion.
  1. +3 −1 lib/client.js
View
4 lib/client.js
@@ -54,7 +54,9 @@ function BinaryClient(socket, options) {
this._socket.addEventListener('close', function(code, message){
var ids = Object.keys(self.streams);
for (var i = 0, ii = ids.length; i < ii; i++) {
- self.streams[ids[i]]._onClose();
+ if(!!self.streams[ids[i]]) {
@dominicklim Collaborator

I don't think we need this check since we're just iterating over stream ids.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ self.streams[ids[i]]._onClose();
+ }
}
self.emit('close', code, message);
@dominicklim Collaborator

It seems like the actual problem is here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
});
Something went wrong with that request. Please try again.