fixes to event encoding and acknowledgement #242
Conversation
|
The last three is of concern. Don't know so far how to remove commits... |
| @@ -414,7 +414,7 @@ Manager.prototype.handleHandshake = function (data, req, res) { | |||
| function writeErr (status, message) { | |||
| if (data.query.jsonp) { | |||
| res.writeHead(200); | |||
| res.end('io.j[' + data.query.jsonp + '](new Error(' + message + '));'); | |||
| res.end('io.j[' + data.query.jsonp + '](new Error(' + JSON.stringify(message) + '));'); | |||
There was a problem hiding this comment.
i'll rather see "' + message + '" here instead of a JSON.stringify.
There was a problem hiding this comment.
what if message is verbatim 'I like to collaborate to "3rd-Eden"'? ;)
There was a problem hiding this comment.
message.replace(/\"/g, '\\"') ;D? I rather prevent as much JSON.stringify's as possible as they are blocking actions.. There is no streaming JSON parser / stringifier yet ;9..
There was a problem hiding this comment.
Here we stringify a short message once for a faulty connection. It's not used in normal message traffic. But I admit that, if we are sure we won't use dquotes in our error messages, your initial proposal saves a couple of ticks.
There was a problem hiding this comment.
I'm going with 3rd-Eden's first approach here.
|
Please, consider applying the last three commits -- they are critical. TIA |
| @@ -329,7 +329,7 @@ Manager.prototype.handleClient = function (data, req) { | |||
| if (count == 1) { | |||
| // initialize the socket for all namespaces | |||
| for (var i in self.namespaces) { | |||
| self.namespaces[i].socket(data.id, true); | |||
| self.namespaces[i].handlePacket(data.id, {type: 'connect'}); | |||
There was a problem hiding this comment.
The way this works changed recently. Please see up-to-date version.
No description provided.