foo & bla example seem to be broken #13

Closed
dscape opened this Issue Dec 27, 2012 · 7 comments

Comments

Projects
None yet
4 participants
Contributor

dscape commented Dec 27, 2012

foo.js should connect to bla.js, etc.

However this doesn't work because foo.js throws the following exception:

/Users/dscape/Desktop/dev/nodejitsu/nssocket/node_modules/eventemitter2/lib/eventemitter2.js:283
          throw arguments[1]; // Unhandled 'error' event
                         ^
Error: NsSocket: sending on a bad socket
    at NsSocket.send (/Users/dscape/Desktop/dev/nodejitsu/nssocket/lib/nssocket.js:101:31)
    at Socket.<anonymous> (/Users/dscape/Desktop/dev/nodejitsu/nssocket/examples/foo.js:10:14)
    at Socket.EventEmitter.emit (events.js:123:20)
    at Object.afterConnect [as oncomplete] (net.js:751:10)

This is because the socket is not considered to be connected: While it is a writable socket it is not readable as no server is created in the foo.js example.

Changing:

this.connected = this.socket.writable && this.socket.readable || false;

for

this.connected = this.socket.writable || this.socket.readable || false;

Would solve the issue, but I'm unaware of other consequences this could have. Thoughts?

Contributor

mmalecki commented Dec 27, 2012

This issue's title is hilarious.

Owner

indexzero commented Dec 27, 2012

Lost comment from @hij1nx found in my Github feed. comments

0x00A commented Dec 27, 2012

That img kind of takes my comment out of context; either way, I recommend using github.com/hij1nx/vines

Owner

indexzero commented Dec 27, 2012

Kinda conflating two things no? Why don't the examples work?

0x00A commented Dec 27, 2012

It depends on why the issue came up. But @dscape's suggestion looks perfectly fine to me. This issue appears to have been introduced with the addition of binary support. We should consider this a review. I will close and look to @dscape for his commit.

@0x00A 0x00A closed this Dec 27, 2012

Owner

indexzero commented Dec 28, 2012

The issue still exists, so we should leave it open yeah? @dscape can reference it in his PR.

@indexzero indexzero reopened this Dec 28, 2012

Owner

indexzero commented Apr 21, 2013

Fixed in #17.

@indexzero indexzero closed this Apr 21, 2013

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