Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

multiple messages in the same "on('data')" #20

Closed
hendricha opened this issue Jun 24, 2016 · 6 comments
Closed

multiple messages in the same "on('data')" #20

hendricha opened this issue Jun 24, 2016 · 6 comments

Comments

@hendricha
Copy link

hendricha commented Jun 24, 2016

if there are three nodes sending messages at the same time to a single node, then while the first .on 'data' event is being handled, the other two messages are added to the stream, thus the next 'data' event will return a concatenated string, which will be ignored, because that can't be json parsed.

@hendricha hendricha changed the title multiple messages in the same "on('data') multiple messages in the same "on('data')" Jun 24, 2016
@albertosantini
Copy link
Contributor

albertosantini commented Dec 8, 2016

Experimenting the same.

It is enough a Bridge and one Node, shouting very quickly.

For instance a "master" node shouts two messages very close to each other.

The "plugin" node doesn't receive the payload, because the two payloads are concatenated and data is ignored due to: https://github.com/nkcmr/flic/blob/master/lib/bridge.js#L55.

Notice }{:

{
    "id": "14bff5d5-4519-4c36-800a-527540e792a5",
    "method": "SHOUT",
    "data": ["argo.streaming", "{ ... my payload ...}\n"],
    "nodeName": "master"
}{
    "id": "4e2d2786-f2a0-46bc-ba85-a2c84e34a082",
    "method": "SHOUT",
    "data": ["argo.streaming", "{ ... my payload ...}\n"],
    "nodeName": "master"
}

That should be two messages.

Any idea?

@albertosantini
Copy link
Contributor

Script to reproduce the issue:

"use strict";

var flic = require("flic");

flic.createBridge();

var node1 = flic.createNode("node1", function (err) {
    var i;

    if (err) {
        console.log(err);
    } else {
        console.log("node1 online!");

        for (i = 0; i < 3; i += 1) {
            node1.shout("target.shout", {
                index: i
            });
        }
    }
});

Set enviroment variable DEBUG:

$ export DEBUG="flic:bridge,flic:node"

Log.

$ node flic-test.js
  flic:node sending IDENT +0ms
  flic:bridge new node (127.0.0.1:50257) +16ms
  flic:bridge node --> bridge: {"id":"c5020b69-4218-4f97-8d8c-1e3f1ff59260","method":"IDENT","data":[],"nodeName":"node1"} +0ms
  flic:bridge sending ACK (id: c5020b69-4218-4f97-8d8c-1e3f1ff59260) +0ms
  flic:node node <-- bridge: {"id":"5ae6d078-9b33-4492-aea7-7dce19822842","method":"ACK","data":["c5020b69-4218-4f97-8d8c-1e3f1ff59260",[null]]} +15ms
  flic:node received ACK from Bridge for message: c5020b69-4218-4f97-8d8c-1e3f1ff59260 +0ms
node1 online!
  flic:bridge node --> bridge: {"id":"a1738215-4124-40ec-b12f-305c705fc8fa","method":"SHOUT","data":["target.shout",{"index":0}],"nodeName":"node1"}{"id":"e318c9a2-c8cb-4177-af4f-bdf9aed50611","method":"SHOUT","data":["target.shout",{"index":1}],"nodeName":"node1"}{"id":"5ad3b247-3b70-4871-b63a-99892a98f90b","method":"SHOUT","data":["target.shout",{"index":2}],"nodeName":"node1"} +0ms
  flic:bridge bridge received invalid data, ignoring... +0ms

Bridge receives invalid data because the three shouts are concatenated in one string and, when it is json parsed, an error is thrown.

@albertosantini
Copy link
Contributor

albertosantini commented Dec 9, 2016

Debugging a bit more, and guessing about setNoDelay, I read the following one:
nodejs/node#906

Then, stealing a snippet from that issue, I tested this one:

"use strict";

var net = require("net");

var port = 4000;

var server = net.createServer(function (socket) {
    socket.setNoDelay(true);
    socket.write("go");
    socket.on("data", function (data) {
        console.log(data);
    });
}).listen(port);

var socket = new net.Socket();

socket.once("connect", socket.setNoDelay);
socket.on("data", function () {
    socket.write("1");
    socket.write("2");
    socket.write("3");
    socket.end();

    server.close();
}).connect(port);

Result:

$ node nodelay.js
<Buffer 31 32 33>

As in flic example above, I would expect three console logs, not only one.

Maybe I am missing something here and I was wondering if, behind the scenes, flic behaviour is due to Node.js. I have not idea if that is expected or not.

@albertosantini
Copy link
Contributor

I am afraid that is expected and it is called "coalescing of TCP packets".

See nodejs/node-v0.x-archive#15443 (comment)

@nkcmr Thougths?

@albertosantini
Copy link
Contributor

After giving a look at node-ipc and jsonsocket projects, I prepared the PR above.
Basically I added an (arbitrary) binary delimeter, '\0'.

If we send a message with a semantic structure, like json, we cannot rely on TCP to maintain the coherence of the message. It is explained very well in the comment link (node-v0.x archive) I provided above.

I hope PR looks good for you.

@nkcmr nkcmr closed this as completed in #22 Dec 11, 2016
@nkcmr
Copy link
Owner

nkcmr commented Dec 11, 2016

Thanks for the investigative work @albertosantini. Will publish a release to NPM with your changes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants