Demo server won't work as expected or work with multiple client connections

[danmactough]

myConnection will be undefined until a connection sends back a text message. https://github.com/scripting/betterWebSocketsDemo/blob/master/server/server.js#L16 So, the everySecond function won't send it's message to a connected client until that client has first sent a text. https://github.com/scripting/betterWebSocketsDemo/blob/master/server/server.js#L7-L8

Bigger concern though I think is that every time a new client sends a text, it's connection becomes myConnection and therefore the everySecond function will only send to the most recently chatty connection.

I think you probably don't want to use myConnection at all. In the everySecond function, you probably want to broadcast to all connections, which you can do. The websockets library you're using manages a connections Array on the Server instance it creates. So you could change https://github.com/scripting/betterWebSocketsDemo/blob/master/server/server.js#L31 to this:

var server = ws.createServer(handleConnection);
server.listen(myPort);

And then in everySecond:

var theMsg = "Hello there #" + ctMessages++;
console.log ("everySecond: sending \"" + theMsg + "\"");
server.connections.forEach(function (connection) {
  connection.sendText (theMsg);
});

[scripting]

I said that I could have done that, in the docs, and I do in the "real" software, but this is just a demo.

I also am aware that I have to send a text to get it primed. You didn't say what the fix is. I need that.

On Saturday, November 28, 2015, Dan MacTough [email protected] wrote:

myConnection will be undefined until a connection sends back a text message. https://github.com/scripting/betterWebSocketsDemo/blob/master/server/server.js#L16 So, the everySecond function won't send it's message to a connected client until that client has first sent a text. https://github.com/scripting/betterWebSocketsDemo/blob/master/server/server.js#L7-L8

Bigger concern though I think is that every time a new client sends a text, it's connection becomes myConnection and therefore the everySecond function will only send to the most recently chatty connection.

I think you probably don't want to use myConnection at all. In the everySecond function, you probably want to broadcast to all connections, which you can do. The websockets library you're using manages a connections Array on the Server instance it creates. So you could change https://github.com/scripting/betterWebSocketsDemo/blob/master/server/server.js#L31 to this:

var server = ws.createServer(handleConnection);server.listen(myPort);

And then in everySecond:

var theMsg = "Hello there #" + ctMessages++;console.log ("everySecond: sending "" + theMsg + """);server.connections.forEach(function (connection) { connection.sendText (theMsg); });

— Reply to this email directly or view it on GitHub https://github.com/scripting/betterWebSocketsDemo/issues/2.

Typed on an iPad with fat fingers.

[danmactough]

Instead of using the text event, just do it within the connection listener, where you attach the other listeners.

[scripting]

Thanks..

That makes sense. The only sample code I had did it the way I was doing it.

I made the change.

https://github.com/scripting/betterWebSocketsDemo/blob/master/server/server.js#L15

Just want to be sure this is what you had in mind.

Dave

On Sat, Nov 28, 2015 at 11:07 AM, Dan MacTough [email protected] wrote:

Instead of using the text event, just do it within the connection listener, where you attach the other listeners.

— Reply to this email directly or view it on GitHub https://github.com/scripting/betterWebSocketsDemo/issues/2#issuecomment-160314808 .

[danmactough]

Exactly

On Sat, Nov 28, 2015 at 11:17 AM, Dave Winer [email protected] wrote:

Thanks.. That makes sense. The only sample code I had did it the way I was doing it. I made the change. https://github.com/scripting/betterWebSocketsDemo/blob/master/server/server.js#L15 Just want to be sure this is what you had in mind. Dave On Sat, Nov 28, 2015 at 11:07 AM, Dan MacTough [email protected] wrote:

Instead of using the text event, just do it within the connection listener, where you attach the other listeners.

— Reply to this email directly or view it on GitHub https://github.com/scripting/betterWebSocketsDemo/issues/2#issuecomment-160314808 .

---

Reply to this email directly or view it on GitHub: https://github.com/scripting/betterWebSocketsDemo/issues/2#issuecomment-160315672

[scripting]

I made it so it could handle multiple simultaneous connections.

One thing surprised me that I could compare two objects in JavaScript, and it actually works.

https://github.com/scripting/betterWebSocketsDemo/blob/master/server/server.js#L14

And I tested the app in all combinations and it works nicely.

Let me know if you spot any more problems.

Dave

On Sat, Nov 28, 2015 at 11:19 AM, Dan MacTough [email protected] wrote:

Exactly

On Sat, Nov 28, 2015 at 11:17 AM, Dave Winer [email protected] wrote:

Thanks.. That makes sense. The only sample code I had did it the way I was doing it. I made the change.

https://github.com/scripting/betterWebSocketsDemo/blob/master/server/server.js#L15 Just want to be sure this is what you had in mind. Dave On Sat, Nov 28, 2015 at 11:07 AM, Dan MacTough <[email protected]

wrote:

Instead of using the text event, just do it within the connection listener, where you attach the other listeners.

— Reply to this email directly or view it on GitHub < https://github.com/scripting/betterWebSocketsDemo/issues/2#issuecomment-160314808

.

---

Reply to this email directly or view it on GitHub:

https://github.com/scripting/betterWebSocketsDemo/issues/2#issuecomment-160315672

— Reply to this email directly or view it on GitHub https://github.com/scripting/betterWebSocketsDemo/issues/2#issuecomment-160315758 .

[danmactough]

One thing surprised me that I could compare two objects in JavaScript, and it actually works.

:heart: One of my favorite things!

Looks good. The nodejs-websockets library already handles keeping a reference to all the connections, though. You don't need to do that yourself. I see you want each connection to have it's own counter, but you could just tack that onto the connection itself like so:

function handleConnection (conn) {
    conn.ctMessages = 0;
    console.log ("New connection received.");
    // ...
}

And if you keep a reference to the websocket server:

var server = ws.createServer(handleConnection);
server.listen(myPort);

Then the everySecond function could just be:

function everySecond () {
    for (var i = 0; i < server.connections.length; i++) {
        var theConnection = server.connections [i];
        var theMsg = "Hello there #" + theConnection.ctMessages++;
        console.log ("sending \"" + theMsg + "\" to the app on socket #" + i);
        theConnection.sendText (theMsg);
        }
    }
}

And you wouldn't need addSocketToArray/deleteSocketFromArray.

[scripting]

I made this change too, but realize that it only works if there's only one kind of socket opened for the app.

In the "real" app -- nodeStorage, I'll probably still use the array, because I'd rather not have to deal with that problem later if there's another kind of WebSockets connection.

But for this demo it should be okay.

[scripting]

I just finished the "real" implementation and in the end I used the connection array of nodejs-websocket. I can tell it's my connection because I add an object to the connection struct, with my data in it. It does simplify things, quite a bit, to not duplicate the structure, and I'm satisfied I'll be able to evolve it.

[scripting]

Next websockets question.

On the server, I get an error event, ECONNRESET.

No indication of the IP address of the other side of the connection, i.e. conn.socket.remoteAddress is undefined.

The question: Should I just close the connection? Is it serving any purpose to keep it around? Or should I leave it alone, it's just an intermittent thing?

They happen fairly steadily, more often after the server has been running for a while.